[astro-useragent] update docs to cover both server and browser usage#1127
[astro-useragent] update docs to cover both server and browser usage#1127
Conversation
🦋 Changeset detectedLatest commit: 16d8a5e The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughUpdated astro-useragent docs to document both server- and client-side usage, inlined and stylistically adjusted the Google Tag Manager snippet, and modified CI workflows to add Node 24, adjust pnpm setup and installation steps, and include NPM_TOKEN in the release job. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser
participant Server
participant AstroUA as "astro-useragent"
rect rgb(214,245,230)
note right of Browser #lightgreen: Client-side flow
Browser->>AstroUA: useUserAgent(navigator.userAgent)
AstroUA-->>Browser: { source: "client", isMobile, browser }
end
rect rgb(245,230,214)
note right of Server #lightorange: Server-side flow (SSR/API)
Server->>AstroUA: parse(Astro.request.headers['user-agent'])
AstroUA-->>Server: { source: "server", isMobile, browser }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/astro-useragent/README.md (4)
123-125: Fix redirect URL to absolute form.
Response.redirect('mobile.mysite.com', 307)yields a relative Location header. Use an absolute URL.- if (isMobile) { - return Response.redirect('mobile.mysite.com', 307); - } + if (isMobile) { + return Response.redirect('https://mobile.mysite.com/', 307); + }
69-69: Heading consistency nit.Prefer “Usage on the server (SSR)” to mirror the browser section.
-### Using on the server (SSR) +### Usage on the server (SSR)
87-87: Code fence filename hint for GitHub rendering.Use the
title=hint so GitHub shows the filename while preserving syntax highlighting.-```astro layout.astro +```astro title=src/pages/layout.astro
91-93: Handle missing UA header defensively.Some proxies strip
user-agent. A null-safe fallback avoids surprises.-const uaString = Astro.request.headers.get('user-agent'); +const uaString = Astro.request.headers.get('user-agent') ?? '';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
package.json(1 hunks)packages/astro-useragent/README.md(4 hunks)
🔇 Additional comments (2)
packages/astro-useragent/README.md (2)
40-66: Client example looks good.Clear, minimal example; works with Astro/Vite bundling for bare module specifiers.
81-82: Verify/update Astro SSR docs anchor
The SSR guide is now at /guides/on-demand-rendering/ and contains "Astro.request.headers", but the fragment "#astrorequestheaders" doesn't appear to be the canonical ID — update the link in packages/astro-useragent/README.md (lines 81–82) to point to the page URL or the exact current anchor for the "Astro.request.headers" section.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/astro-gtm/src/GoogleTagManager.astro (1)
61-84: Add CSP nonce support to avoid breakage under strict CSPInline script + dynamic script injection will be blocked without a nonce in common CSP configs. Propagate a nonce to both the inline tag and the injected GTM script.
Apply this diff within the selected range:
- <script define:vars={{ gtmId, dataLayerName, dataLayer, auth, preview }} is:inline> + <script define:vars={{ gtmId, dataLayerName, dataLayer, auth, preview, nonce }} is:inline nonce={nonce}> var w = window; var d = document; @@ - var s = d.createElement("script"); + var s = d.createElement("script"); + // Propagate CSP nonce to the injected script (works with strict CSP) + if (d.currentScript && d.currentScript.nonce) { + s.nonce = d.currentScript.nonce; + } else if (nonce) { + s.setAttribute("nonce", nonce); + }And outside these lines, add the prop and wire it up:
export interface Props { @@ preview?: string; + /** + * Optional CSP nonce for the inline and injected GTM scripts + */ + nonce?: string; } @@ const { gtmId, dataLayer = {}, dataLayerName = "dataLayer", includeNoScript = true, enableInDevMode = false, auth, preview, + nonce, } = Astro.props;
🧹 Nitpick comments (7)
packages/astro-gtm/src/GoogleTagManager.astro (3)
73-83: Include gtm_cookies_win for GTM Preview URLs (parity with GTM docs)Preview/auth is appended but gtm_cookies_win is missing; many GTM preview setups require it.
Apply this diff:
- var ath = auth ? `>m_auth=${auth}` : ""; - var prv = preview ? `>m_preview=${preview}` : ""; + var ath = auth ? `>m_auth=${auth}` : ""; + var prv = preview ? `>m_preview=${preview}` : ""; + var cw = preview ? `>m_cookies_win=x` : ""; @@ - s.src = `https://www.googletagmanager.com/gtm.js?id=${gtmId}${dl}${ath}${prv}`; + s.src = `https://www.googletagmanager.com/gtm.js?id=${gtmId}${dl}${ath}${prv}${cw ? `&${cw}` : ""}`;And in the noscript iframe:
- src={`https://www.googletagmanager.com/ns.html?id=${gtmId}${auth ? `>m_auth=${auth}` : ''}${preview ? `>m_preview=${preview}` : ''}`} + src={`https://www.googletagmanager.com/ns.html?id=${gtmId}${auth ? `>m_auth=${auth}` : ''}${preview ? `>m_preview=${preview}>m_cookies_win=x` : ''}`}Also applies to: 86-94
66-71: Confirm desired push order vs. canonical snippetYou push custom data before the "gtm.js" bootstrap event. Canonical GTM snippet pushes gtm.start first. If you intended canonical ordering, swap these; otherwise, please confirm.
Proposed reorder:
- w[dataLayerName].push(dataLayer); - w[dataLayerName].push({ + w[dataLayerName].push({ "gtm.start": new Date().getTime(), event: "gtm.js", }); + if (dataLayer && Object.keys(dataLayer).length) { + w[dataLayerName].push(dataLayer); + }
62-63: Prefer const for immutablesLocals are never reassigned; const improves clarity and avoids accidental mutation.
Apply this diff:
- var w = window; - var d = document; + const w = window; + const d = document; @@ - var dl = dataLayerName !== "dataLayer" ? "&l=" + dataLayerName : ""; - var ath = auth ? `>m_auth=${auth}` : ""; - var prv = preview ? `>m_preview=${preview}` : ""; + const dl = dataLayerName !== "dataLayer" ? "&l=" + dataLayerName : ""; + const ath = auth ? `>m_auth=${auth}` : ""; + const prv = preview ? `>m_preview=${preview}` : ""; @@ - var s = d.createElement("script"); + const s = d.createElement("script");Also applies to: 74-79
packages/astro-useragent/README.md (4)
87-105: Minor: file name in title implies a layout, not a pageIf this is meant as a page, consider naming it index.astro in docs to reduce confusion. No code change required.
139-139: Tiny grammar + inclusive language nit“setup” → “set up”; “he is” → “they are”.
Apply this diff:
-We have also setup an example repository available here: [example-useragent](../../apps/example-useragent) +We have also set up an example repository available here: [example-useragent](../../apps/example-useragent)Optional wording above the redirect snippet:
-In the example below, an API route is used to redirect a user to a different mobile page when he is using a mobile client, otherwise it serves the normal content. +In the example below, an API route redirects users to a mobile page when they are on a mobile client; otherwise it serves the normal content.
188-195: Optional: note UA reduction and Client HintsBrowsers (e.g., Chromium) reduce UA strings; consider a short note recommending UA‑CH if high fidelity is needed.
119-135: Handle null user-agent header & align handler casing with Astro v5
- Guard the header: use a safe fallback when calling useUserAgent — apply the diff below.
-export async function get({ request }: APIContext) { - const uaString = request.headers.get('user-agent'); - const { isMobile } = useUserAgent(uaString); +export async function get({ request }: APIContext) { + const uaString = request.headers.get('user-agent'); + const { isMobile } = useUserAgent(uaString ?? '');
- Handler casing: repo package.json is using Astro ^5.13.7; Astro v5 docs/examples use an uppercase GET export. Update the handler export in the README snippet and the example file to match (export async function GET(...) or export const GET = ...). Files to change: packages/astro-useragent/README.md (lines 119–135) and apps/example-useragent/src/pages/hello.ts (currently exports
get). (docs.astro.build)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/astro-gtm/src/GoogleTagManager.astro(1 hunks)packages/astro-useragent/README.md(5 hunks)
🔇 Additional comments (4)
packages/astro-useragent/README.md (4)
9-15: Clear dual‑mode overviewGood clarification that the lib works both server‑side and client‑side.
40-69: Client-side example reads wellSolid, minimal example for browser usage.
71-79: SSR guidance LGTMEnables server parsing without over-prescribing config.
139-139: Example link verified — no action required. The path apps/example-useragent exists in the monorepo.
mhdcodes
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
.github/workflows/release.yml (2)
17-18: Scope NODE_OPTIONS to the publishing step, not the whole job.Job-wide
NODE_OPTIONS: --no-deprecationhides useful warnings during checkout/install as well. Limit it to the Changesets step env.permissions: id-token: write contents: write pull-requests: write - env: - NODE_OPTIONS: --no-deprecation steps: - uses: actions/checkout@v5 - - uses: pnpm/action-setup@v4 - with: - run_install: true + # 1) Ensure Node is set before any installs + - name: Setup Node.js + uses: actions/setup-node@v5 + with: + node-version: '22' + cache: 'pnpm' + cache-dependency-path: pnpm-lock.yaml + + # 2) Then setup pnpm and install + - uses: pnpm/action-setup@v4 + with: + run_install: true @@ - name: Create Release Pull Request or Publish to npm id: changesets uses: changesets/action@v1 with: version: pnpm run changeset:version publish: pnpm run changeset:publish - - env: + env: + NODE_OPTIONS: --no-deprecation NPM_CONFIG_PROVENANCE: true GITHUB_TOKEN: ${{ github.token }} NPM_TOKEN: ${{ secrets.NPM_TOKEN }}Also applies to: 31-42
21-29: Install Node before pnpm install to avoid engine mismatch.Currently
pnpm/action-setupruns install beforesetup-node, so dependency resolution may occur under the runner’s default Node instead of v22.The diff above reorders steps and adds
cache-dependency-pathfor more reliable caching.packages/astro-gtm/src/GoogleTagManager.astro (4)
74-81: Encode all URL parameters (correctness, safety).
gtmId,dataLayerName,auth, andpreviewshould be URL-encoded to avoid malformed URLs and edge-case breakage.- var dl = dataLayerName !== "dataLayer" ? "&l=" + dataLayerName : ""; - var ath = auth ? `>m_auth=${auth}` : ""; - var prv = preview ? `>m_preview=${preview}` : ""; + var dl = dataLayerName !== "dataLayer" ? "&l=" + encodeURIComponent(dataLayerName) : ""; + var ath = auth ? `>m_auth=${encodeURIComponent(auth)}` : ""; + var prv = preview ? `>m_preview=${encodeURIComponent(preview)}` : ""; @@ - s.src = `https://www.googletagmanager.com/gtm.js?id=${gtmId}${dl}${ath}${prv}`; + s.src = `https://www.googletagmanager.com/gtm.js?id=${encodeURIComponent(gtmId)}${dl}${ath}${prv}`;
86-95: Mirror encoding in the noscript URL.Keep URL construction consistent with the script tag.
- src={`https://www.googletagmanager.com/ns.html?id=${gtmId}${auth ? `>m_auth=${auth}` : ''}${preview ? `>m_preview=${preview}` : ''}`} + src={`https://www.googletagmanager.com/ns.html?id=${encodeURIComponent(gtmId)}${auth ? `>m_auth=${encodeURIComponent(auth)}` : ''}${preview ? `>m_preview=${encodeURIComponent(preview)}` : ''}`}
78-83: Avoid double-inserting GTM if the component is rendered multiple times.Guard by ID and tag the created script for idempotency.
- var s = d.createElement("script"); + if (d.getElementById(`gtm-js-${gtmId}`)) { return; } + var s = d.createElement("script"); + s.id = `gtm-js-${gtmId}`;
2-43: Optional: Add CSP nonce plumbing (inline + injected script).Helps teams with strict CSP to run GTM without relying on 'unsafe-inline'.
export interface Props { @@ preview?: string; + /** + * Optional CSP nonce applied to inline and injected scripts + */ + nonce?: string; } @@ } = Astro.props; @@ -{shouldRender && ( - <script define:vars={{ gtmId, dataLayerName, dataLayer, auth, preview }} is:inline> +{shouldRender && ( + <script define:vars={{ gtmId, dataLayerName, dataLayer, auth, preview, nonce }} is:inline nonce={nonce}> @@ - d.head.appendChild(s); + if (nonce) { s.nonce = nonce; } + d.head.appendChild(s); </script> )}Also applies to: 60-84
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/release.yml(1 hunks)packages/astro-gtm/src/GoogleTagManager.astro(1 hunks)packages/astro-useragent/README.md(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/astro-useragent/README.md
🔇 Additional comments (2)
packages/astro-gtm/src/GoogleTagManager.astro (1)
61-64: Inline script usage LGTM.
is:inlineis correct for GTM; scopingwindow/documentvia locals keeps the snippet compact..github/workflows/release.yml (1)
31-42: Action required — confirm NPM_TOKEN; changesets scripts verified.
- changeset:version and changeset:publish are present in package.json (lines 17–18).
- Confirm NPM_TOKEN is set in repository or organization secrets for this workflow — secrets can't be verified from this environment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
.github/workflows/node.js.yml (1)
28-31: Pin pnpm and use a frozen, cached install for reproducibility.Lock pnpm to a known version and pass --frozen-lockfile (and prefer-offline) to make installs deterministic and faster.
Apply:
- - name: Setup pnpm - uses: pnpm/action-setup@v4 - with: - run_install: true + - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9.12.0 + run_install: | + recursive: true + args: [--frozen-lockfile, --prefer-offline].github/workflows/release.yml (4)
17-18: Scope NODE_OPTIONS to the publish step to avoid masking useful warnings elsewhere.Set --no-deprecation only where you need it (publishing), not for the entire job.
Apply:
- env: - NODE_OPTIONS: --no-deprecation + # (Optional) keep job env clean; set at step-level insteadAnd add to the changesets step:
- name: Create Release Pull Request or Publish to npm id: changesets uses: changesets/action@v1 with: version: pnpm run changeset:version publish: pnpm run changeset:publish env: + NODE_OPTIONS: --no-deprecation NPM_CONFIG_PROVENANCE: true GITHUB_TOKEN: ${{ github.token }} NPM_TOKEN: ${{ secrets.NPM_TOKEN }}
22-27: Add npm registry-url for provenance/publish auth hardening.This ensures .npmrc is configured by the action and avoids flaky auth.
Apply:
- name: Setup Node.js uses: actions/setup-node@v5 with: node-version: '22' cache: 'pnpm' + registry-url: 'https://registry.npmjs.org'
20-21: Fetch full history to keep Changesets happy.Some Changesets flows rely on tags/history. Safer to fetch all commits.
Apply:
- - uses: actions/checkout@v5 + - uses: actions/checkout@v5 + with: + fetch-depth: 0
28-31: Pin pnpm and enforce frozen installs.For consistent releases across runners.
Apply:
- name: Setup pnpm uses: pnpm/action-setup@v4 with: - run_install: true + version: 9.12.0 + run_install: | + recursive: true + args: [--frozen-lockfile, --prefer-offline].github/workflows/pkgpr.yml (2)
20-23: Pin pnpm and use --frozen-lockfile for deterministic CI.Same rationale as other workflows; keeps pkgpr builds reproducible.
Apply:
- name: Setup pnpm uses: pnpm/action-setup@v4 with: - run_install: true + version: 9.12.0 + run_install: | + recursive: true + args: [--frozen-lockfile, --prefer-offline]
11-28: Optional hardening: fail-fast off and job timeout.Prevents one matrix failure from canceling others and avoids hung jobs.
If you add a matrix later, consider:
jobs: build: runs-on: ubuntu-latest + timeout-minutes: 20
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/node.js.yml(1 hunks).github/workflows/pkgpr.yml(1 hunks).github/workflows/release.yml(1 hunks)
🔇 Additional comments (3)
.github/workflows/node.js.yml (1)
17-17: Good call adding Node 24 to the matrix.This broadens coverage and will surface any upcoming LTS issues early.
If you enforce Node engines, verify they include 24.x to avoid CI vs. publish mismatches.
.github/workflows/pkgpr.yml (2)
14-19: Enabling pnpm cache on setup-node is spot on.This reduces cold-install times in pkgpr runs.
1-28: Repo health — packageManager / lockfile / engines
- pnpm-lock.yaml present at repo root (OK).
- Root package.json contains "packageManager": "pnpm@9.15.9"; workspace package.json files (apps/* and packages/*) do not have packageManager entries.
- engines.node check failed due to ripgrep regex parse error so engines.node could not be verified across packages. Run this to verify:
fd -HI package.json | while read -r f; do echo "==> $f"; jq -r '.engines.node // "(no engines.node)"' "$f"; done- Two .nvmrc files found: apps/example-purgecss-ssr/.nvmrc and apps/example-useragent/.nvmrc — consolidate Node-version single source (move .nvmrc to repo root or add engines.node consistently).
Actions: add packageManager to workspace package.jsons or confirm reliance on root packageManager; consolidate Node version source; re-run the engines check above.
commit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.github/workflows/release.yml (2)
27-28: Optional: pin pnpm version to avoid toolchain driftpnpm/action-setup@v4 defaults to “latest”. Consider pinning for reproducibility.
Add a version pin to the pnpm setup step:
- uses: pnpm/action-setup@v4 with: version: 9.12.0 # or your vetted version
27-28: Provenance: ensure npm is configured for OIDCYou’ve set NPM_CONFIG_PROVENANCE=true and id-token: write; add registry-url so npm auth and provenance are wired consistently across environments.
Proposed adjustment to the Setup Node step:
- name: Setup Node.js uses: actions/setup-node@v5 with: node-version: '22' cache: 'pnpm' registry-url: 'https://registry.npmjs.org'.github/workflows/pkgpr.yml (2)
13-13: Setup order and version pin for pnpmOrder is fine, but pinning pnpm reduces CI flakiness.
- uses: pnpm/action-setup@v4 with: version: 9.12.0
15-20: Align Node versioning across workflowsThis uses Node 22; other workflows reportedly add 24.x. Decide on a policy (matrix vs single pinned) and align for consistency (or read from .nvmrc).
Example options:
- Use .nvmrc:
with: node-version-file: '.nvmrc' cache: 'pnpm'
- Or a matrix (22.x, 24.x) if you want both covered here too.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/node.js.yml(1 hunks).github/workflows/pkgpr.yml(1 hunks).github/workflows/release.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/node.js.yml
🔇 Additional comments (2)
.github/workflows/release.yml (1)
27-28: Explicit install with frozen lockfile — good moveThis replaces implicit pnpm install and makes caching behavior predictable. LGTM.
If the repo uses a workspace with a non-root pnpm-lock.yaml, set cache-dependency-path accordingly in Setup Node to maximize cache hits.
.github/workflows/pkgpr.yml (1)
21-22: Install + build steps look goodFrozen lockfile and explicit build are appropriate for deterministic CI.
1 participant
Summary by CodeRabbit