Skip to content

feat(router): add typed file routes with codegen and Standard Schema validation with JSON-LD support#2125

Merged
brandonroberts merged 2 commits into
analogjs:alphafrom
benpsnyder:feat/file-based-routing-type-safe
Mar 25, 2026
Merged

feat(router): add typed file routes with codegen and Standard Schema validation with JSON-LD support#2125
brandonroberts merged 2 commits into
analogjs:alphafrom
benpsnyder:feat/file-based-routing-type-safe

Conversation

@benpsnyder

@benpsnyder benpsnyder commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

PR Checklist

Closes #2044

Affected scope

  • Primary scope: router
  • Secondary scopes: vite-plugin-routes, platform, content

Recommended merge strategy for maintainer [optional]

  • Squash merge
  • Rebase merge
  • Other

Commit preservation note [optional]

Crosses multiple scopes because typed file routes require coordinated changes across codegen, platform opt-in, router runtime, and content metadata. Squash merge is still preferred.

What is the new behavior?

Typed file-route codegen@analogjs/vite-plugin-routes scans .page.ts and content files, generates src/routeTree.gen.ts with typed route table, route tree metadata, and JSON-LD manifest.

Standard Schema validationdefineAction() and defineApiRoute() validate params, query, and body using any Standard Schema library (Valibot, Zod, ArkType). Content frontmatter validation via contentFileResource({ schema }).

Router runtimeroutePath() type-safe URL builder, buildUrl() with error on missing required params, injectTypedParams()/injectTypedQuery() signal accessors, routeLink pipe.

Platform integrationexperimental.typedRouter: true in analog() config enables codegen via @analogjs/vite-plugin-routes. JSON-LD manifest combined into routeTree.gen.ts.

Vite 6–8 compat — build configs use rollupOptions (cross-version) with commented rolldownOptions for future Vite 8-only migration.

Test plan

  • nx format:check
  • pnpm build
  • pnpm test
  • Manual verification

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Scoped to typed file routes, route contracts, and the supporting platform/runtime/docs work needed to make the feature production-ready.

@netlify

netlify Bot commented Mar 15, 2026

Copy link
Copy Markdown

Deploy Preview for analog-blog ready!

Name Link
🔨 Latest commit 7b77381
🔍 Latest deploy log https://app.netlify.com/projects/analog-blog/deploys/69c34bb5aae4af00089e6b99
😎 Deploy Preview https://deploy-preview-2125--analog-blog.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@benpsnyder

Copy link
Copy Markdown
Contributor Author
image

Observed behavior with dev server HMR route discovery:

Pages that exist before the dev server starts are served correctly, but pages created while the server is running cannot be navigated to.

Repro:

  1. Create test-ben.page.ts in src/app/pages/
  2. Start the dev server with nx run analog-app:serve --no-tui
  3. /test-ben works
  4. While the server is running, create test-brandon.page.ts in the same directory
  5. /test-brandon returns a 404 — does not resolve without restarting the server

Why: The analogjs-router-invalidate-routes plugin (packages/platform/src/lib/router-plugin.ts) listens for Vite's file watcher add events and triggers module graph
invalidation + full reload when a new page file appears. However, the route list itself is built at startup via globSync in the analog-glob-routes plugin's transform hook —
so the invalidation fires a reload, but the re-discovered route set may not include the newly added file if the glob result is cached or the transform doesn't re-run.

@benpsnyder

benpsnyder commented Mar 15, 2026

Copy link
Copy Markdown
Contributor Author
image ## Observed behavior with dev server HMR route discovery: Pages that exist before the dev server starts are served correctly, but pages created while the server is running cannot be navigated to.

Repro:

  1. Create test-ben.page.ts in src/app/pages/
  2. Start the dev server with nx run analog-app:serve --no-tui
  3. /test-ben works
  4. While the server is running, create test-brandon.page.ts in the same directory
  5. /test-brandon returns a 404 — does not resolve without restarting the server

Why: The analogjs-router-invalidate-routes plugin (packages/platform/src/lib/router-plugin.ts) listens for Vite's file watcher add events and triggers module graph
invalidation + full reload when a new page file appears. However, the route list itself is built at startup via globSync in the analog-glob-routes plugin's transform hook —
so the invalidation fires a reload, but the re-discovered route set may not include the newly added file if the glob result is cached or the transform doesn't re-run.

I dug into the hot-added route issue and confirmed the original failure mode is addressed in commit d4d014a (feat(router): add unit tests for route module invalidation on file changes).

What was happening in the first scenario:

  • Analog’s typed route generation was correctly discovering the newly added page file.
  • But the runtime side could still serve stale module state during nx serve if the page was first seen while its contents were incomplete.
  • The fix added invalidation in two places:
    • packages/platform/src/lib/router-plugin.ts: invalidate the changed route/page module and the router modules on route-like file updates.
    • packages/vite-plugin-angular/src/lib/angular-vite-plugin.ts: clear Angular compilation caches on file add/unlink so newly added page files are included in the compilation program.
      There is regression coverage for this in:
  • packages/platform/src/lib/router-plugin.spec.ts
  • packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts
    I also tested enabling useAngularCompilationAPI: true, and that works as a workaround / alternative path because it bypasses the older TypeScript/Ngtsc cached-rootNames compilation flow that was involved in this bug.

Why useAngularCompilationAPI: true helps:

  • The non-Compilation-API path caches tsconfig root file names and incremental compiler state.
  • Hot-added *.page.ts files could be discovered by routing before Angular’s compilation program fully picked them up.
  • With useAngularCompilationAPI: true, the plugin uses performAngularCompilation() instead of that cached rootNames path, so the newly added page is compiled through the newer Angular compilation flow.
    One important caveat:
  • useAngularCompilationAPI: true is not a free switch today. In this repo it disables liveReload compatibility and file replacements inside @analogjs/vite-plugin-angular, so it changes dev behavior in addition to avoiding this bug.
    Current status:
  • The original hot-added route issue is fixed in d4d014a.
  • Enabling useAngularCompilationAPI: true also works and likely avoids this class of issue by taking the newer compilation path.

@benpsnyder

benpsnyder commented Mar 16, 2026

Copy link
Copy Markdown
Contributor Author

useAngularCompilationAPI: liveReload & fileReplacements now work

Commit: 96b75e9

Problem

When experimental.useAngularCompilationAPI was enabled, the plugin force-disabled both liveReload and fileReplacements with console warnings. This meant users who opted into the Compilation API lost two commonly-used features.

The root causes were:

  1. liveReloadperformAngularCompilation created a fresh Angular compilation instance on every rebuild. Without prior state, Angular could never produce templateUpdates (the HMR payloads that drive Analog's live-reload flow).
  2. fileReplacements — Angular's AngularHostOptions already accepts a fileReplacements record, but the plugin never wired it through. The guard simply cleared the array.

Fix

  • Persistent compilation instanceangularCompilation is now kept alive across rebuilds (nullish-coalescing assignment). Modified files are forwarded via sourceFileCache.invalidate() and angularCompilation.update() so Angular scopes its incremental re-analysis correctly. On closeBundle, the instance is torn down to avoid memory leaks.
  • HMR mappingtemplateUpdates from compilationResult are decoded by mapTemplateUpdatesToFiles() (Angular encodes them as encodedPath@ClassName) and attached to each EmitFileResult as hmrUpdateCode / hmrEligible. Component class names are registered in the classNames map so the existing angular:component-update WebSocket flow picks them up.
  • File replacements passthroughtoAngularCompilationFileReplacements() converts Analog's browser-style { replace, with } entries into the Record<string, string> shape Angular's host expects. SSR-only replacements ({ replace, ssr }) are intentionally excluded — they remain on the Vite runtime side.
  • Guard removal — the liveReload and fileReplacements force-disable blocks are removed since both features now work with the Compilation API.

Tests

toAngularCompilationFileReplacements (5 cases):

  • Maps browser { replace, with } entries and resolves relative paths against workspace root
  • Filters out SSR-only { replace, ssr } entries
  • Returns undefined for empty input
  • Returns undefined when all entries are SSR-only
  • Passes absolute paths through without resolution
  • Handles mixed absolute/relative path pairs

mapTemplateUpdatesToFiles (5 cases):

  • Decodes Angular's encodedPath@ClassName keys back to absolute paths with class name and HMR code
  • Returns an empty map for undefined input
  • Returns an empty map for empty map input
  • Defaults className to '' when the update ID has no @ separator
  • Preserves multiple updates across different files

What's unchanged

  • The v20.1+ Angular version gate stays in place. The workspace is on Angular 21.1.1 so it's not a blocker, but the gate is tied to a private API and should be revisited with version-matrix testing rather than removed speculatively.

@benpsnyder

Copy link
Copy Markdown
Contributor Author

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown
✅ Actions performed

Reviews resumed.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

♻️ Duplicate comments (1)
packages/router/src/lib/route-path.ts (1)

192-201: ⚠️ Potential issue | 🟠 Major

Reject empty arrays for required catch-all params.

{ slug: [] } currently joins to '', and the later slash cleanup turns /docs/[...slug] into /docs instead of surfacing missing input. Required catch-all segments should fail fast here too.

🧭 Suggested fix
     url = url.replace(/\[\.\.\.([^\]]+)\]/g, (_, name) => {
       const value = params[name];
-      if (value == null) {
+      if (value == null || (Array.isArray(value) && value.length === 0)) {
         throw new Error(
           `Missing required catch-all param "${name}" for path "${path}"`,
         );
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/router/src/lib/route-path.ts` around lines 192 - 201, In the
url.replace handler for catch-all params (the function using params[name] in
route-path.ts), add a check to reject empty arrays: if value is an Array and
value.length === 0, throw the same Missing required catch-all param error
(referencing name and path) instead of joining to an empty string; keep the
existing Array.isArray branch to encode/join non-empty arrays. This ensures
required catch-all segments like `[...slug]` fail fast when params[name] is [].
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/router/src/lib/inject-navigate.ts`:
- Around line 28-35: The returned navigate function's spread signature allows a
NavigationBehaviorOptions object to be structurally accepted as route options
and dropped; fix this by adding explicit function overloads for the returned
function (one overload for calling with only RoutePathArgs<P> and another
overload that requires a separate NavigationBehaviorOptions as the final
argument) so callers must pass extras as a distinct parameter; update the
implementation of the returned function (the closure returned in
inject-navigate.ts) to read extras only from the explicit extras overload
position (e.g., treat the last argument as extras only when the overload that
includes NavigationBehaviorOptions is selected) and add a regression test
asserting that navigate('/about', { replaceUrl: true }) does not lose extras
(and that navigate('/about', undefined, { replaceUrl: true }) still works).

In `@packages/router/src/lib/inject-typed-params.ts`:
- Around line 17-48: Replace the local parser and strict-check with the shared
route parser so optional catch-all params aren't treated as required: import and
call the shared extractRouteParams(...) instead of extractParamNames(...) inside
assertRouteMatch (keep the function name assertRouteMatch and the same
signature), use the returned param descriptors to derive expected params and
only warn when a param is required/missing (skip optional or optional-catch-all
descriptors like [[...slug]]), and remove or deprecate extractParamNames so the
runtime assertion matches the manifest-generated route model.

In `@packages/router/src/lib/route-path.ts`:
- Around line 75-80: RoutePathOptions currently hardcodes query as
Record<string,string|string[]|undefined>, losing route-specific narrowing;
change both occurrences of the query property inside RoutePathOptions to use the
route-specific type by replacing the hardcoded shape with RouteQueryOutput<P>
(keeping the optional flag), analogous to how params uses RouteParamsOutput<P>,
so the two branches become query?: RouteQueryOutput<P> and ensure the generic P
is used in both branches where params already uses RouteParamsOutput<P>.
- Around line 72-82: The type check treating "no params" uses Params extends
Record<string, never> which misses cases where all params are optional (e.g., {
category?: string[] }); update the conditional in RoutePathOptions and
RoutePathArgs to detect "all-optional" params by checking keyof Required<Params>
extends never (i.e., replace the Params extends Record<string, never> branch
with a check like keyof Required<Params> extends never) so routes whose params
are present but all optional are treated as no-params and the options/args
become optional accordingly; apply this change to the conditional branches that
currently reference Params (types RoutePathOptions and RoutePathArgs /
AnalogRouteTable usage).

In `@packages/vite-plugin-routes/src/lib/route-file-discovery.ts`:
- Around line 39-46: joinDir currently prepends workspaceRoot for inputs
starting with '/' or resolving relative paths but incorrectly double-prepends
when entries in additionalPagesDirs/additionalContentDirs are already full
normalized paths; update joinDir to follow the three-way dispatch used by
normalizeWatchedDir: if the incoming dir already starts with workspaceRoot
return normalizePath(dir) directly; else if dir startsWith('/') return
normalizePath(`${workspaceRoot}${dir}`); otherwise return
normalizePath(resolve(workspaceRoot, dir)); apply this logic to where
additionalPagesRoots/additionalContentRoots are computed so already-normalized
`${workspaceRoot}/...` entries are not modified.

In `@packages/vite-plugin-routes/src/lib/typed-routes-plugin.ts`:
- Around line 177-200: The JSON-LD entries are built from raw
routeFiles/contentFiles which can conflict with the collision resolution in
generateRouteManifest; update generate() so collision precedence from manifest
is applied before calling buildJsonLdEntries (e.g., compute the set of canonical
file paths from manifest.routes and pass only those canonical files or pass
manifest into buildJsonLdEntries) so jsonLdEntries honors the same winner
selection as manifest.routes; adjust the call that currently uses
(resolveDiscoveredFile, routeFiles, contentFiles) to use the manifest-filtered
file list or manifest-aware logic (affecting generate(), buildJsonLdEntries, and
any use of jsonLdEntries.map(entry => entry.routePath)).
- Around line 121-175: ensureEntryImport currently mutates src/main.ts or
src/main.server.ts even during read-only verification runs; modify
ensureEntryImport (or call sites like buildStart which invokes generate() then
ensureEntryImport) to skip any file writes when resolvedOptions.verify or
resolvedOptions.verifyOnBuild (or a similar "verify" flag in resolvedOptions) is
true — i.e., compute specifier and detect existing import as now, but before
writeFileSync/console.log/warn return early if in verify mode; apply the same
guard to the other mutation site around lines 269-271 so verification/freshness
checks remain read-only.

---

Duplicate comments:
In `@packages/router/src/lib/route-path.ts`:
- Around line 192-201: In the url.replace handler for catch-all params (the
function using params[name] in route-path.ts), add a check to reject empty
arrays: if value is an Array and value.length === 0, throw the same Missing
required catch-all param error (referencing name and path) instead of joining to
an empty string; keep the existing Array.isArray branch to encode/join non-empty
arrays. This ensures required catch-all segments like `[...slug]` fail fast when
params[name] is [].

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 37a03574-48df-4953-8c87-bac824b50b1e

📥 Commits

Reviewing files that changed from the base of the PR and between e733189 and 4aec881.

⛔ Files ignored due to path filters (2)
  • .dagger/src/index.ts is excluded by none and included by none
  • .nxignore is excluded by none and included by none
📒 Files selected for processing (44)
  • .github/pr-scope-map.json
  • .github/workflows/ci.yml
  • apps/analog-app-e2e/playwright.config.ts
  • apps/analog-app/project.json
  • apps/analog-app/src/main.ts
  • apps/docs-app/docs/features/data-fetching/validation.md
  • apps/docs-app/docs/features/routing/typed-routes.md
  • apps/tanstack-query-app-e2e/playwright.config.ts
  • apps/tanstack-query-app/project.json
  • packages/content-plugin/project.json
  • packages/content-plugin/tsdown.config.ts
  • packages/content/project.json
  • packages/create-analog/project.json
  • packages/platform/src/lib/options.ts
  • packages/router/manifest/src/index.ts
  • packages/router/project.json
  • packages/router/src/index.ts
  • packages/router/src/lib/experimental.ts
  • packages/router/src/lib/inject-navigate.spec.ts
  • packages/router/src/lib/inject-navigate.ts
  • packages/router/src/lib/inject-typed-params.spec.ts
  • packages/router/src/lib/inject-typed-params.ts
  • packages/router/src/lib/route-generation.integration.spec.ts
  • packages/router/src/lib/route-manifest.spec.ts
  • packages/router/src/lib/route-path.spec.ts
  • packages/router/src/lib/route-path.ts
  • packages/storybook-angular/src/lib/preset.spec.ts
  • packages/vite-plugin-routes/README.md
  • packages/vite-plugin-routes/src/index.ts
  • packages/vite-plugin-routes/src/lib/json-ld-manifest-plugin.spec.ts
  • packages/vite-plugin-routes/src/lib/json-ld-manifest-plugin.ts
  • packages/vite-plugin-routes/src/lib/route-file-discovery.spec.ts
  • packages/vite-plugin-routes/src/lib/route-file-discovery.ts
  • packages/vite-plugin-routes/src/lib/typed-routes-plugin.spec.ts
  • packages/vite-plugin-routes/src/lib/typed-routes-plugin.ts
  • packages/vite-plugin-routes/tsconfig.lib.json
  • packages/vite-plugin-routes/vite.config.lib.ts
  • packages/vitest-angular-tools/collection.json
  • packages/vitest-angular-tools/project.json
  • packages/vitest-angular-tools/vite.config.lib.ts
  • packages/vitest-angular/project.json
  • tools/scripts/build-lib.mts
  • tools/scripts/verify-package-artifacts.mts
  • tools/scripts/verify-route-freshness.mts
✅ Files skipped from review due to trivial changes (10)
  • packages/router/project.json
  • packages/vite-plugin-routes/tsconfig.lib.json
  • packages/vite-plugin-routes/src/lib/route-file-discovery.spec.ts
  • packages/vite-plugin-routes/README.md
  • packages/vite-plugin-routes/src/index.ts
  • apps/docs-app/docs/features/routing/typed-routes.md
  • packages/router/src/lib/route-path.spec.ts
  • apps/docs-app/docs/features/data-fetching/validation.md
  • packages/router/src/lib/route-generation.integration.spec.ts
  • packages/router/src/lib/experimental.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • apps/tanstack-query-app-e2e/playwright.config.ts
  • .github/pr-scope-map.json
  • packages/content/project.json
  • packages/create-analog/project.json
  • packages/vite-plugin-routes/src/lib/json-ld-manifest-plugin.ts
  • packages/vite-plugin-routes/src/lib/json-ld-manifest-plugin.spec.ts
  • packages/router/manifest/src/index.ts

Comment thread packages/router/src/lib/inject-navigate.ts Outdated
Comment thread packages/router/src/lib/inject-typed-params.ts Outdated
Comment thread packages/router/src/lib/route-path.ts
Comment thread packages/router/src/lib/route-path.ts Outdated
Comment thread packages/platform/src/lib/route-file-discovery.ts
Comment thread packages/platform/src/lib/typed-routes-plugin.ts
Comment thread packages/platform/src/lib/typed-routes-plugin.ts
@brandonroberts

Copy link
Copy Markdown
Member

Pitfall: Unnecessary package separation adds complexity

The dependency chain for typed route codegen is:

@analogjs/platform
  └─ dynamically imports @analogjs/vite-plugin-routes (workspace:^)
       └─ imports from @analogjs/router/manifest

@analogjs/vite-plugin-routes has exactly one consumer: platform's route-generation-plugin.ts. It's not designed for standalone use — it needs the platform's options (additionalPagesDirs, workspaceRoot, etc.) to function. The dynamic import in the platform wrapper exists solely to work around Nx module boundary lint rules and test mocking, not because there's a real architectural boundary.

@analogjs/router/manifest is a secondary entry point of @analogjs/router containing pure build-time codegen utilities (filenameToRoutePath, generateRouteManifest, generateRouteTableDeclaration, etc.). Its only consumers are the two files in vite-plugin-routes. These utilities have zero Angular runtime dependencies, yet they're packaged inside @analogjs/router which peer-depends on @angular/core, @angular/router, etc.

Both could live in @analogjs/platform since:

  1. Platform is already the sole entry point that wires them together
  2. Platform is already a build-time Vite plugin package, not an Angular runtime package
  3. It would eliminate the dynamic import workaround and the Nx boundary gymnastics
  4. Build-time codegen logic doesn't belong in @analogjs/router (a runtime package)

The current separation creates: an extra published package (@analogjs/vite-plugin-routes at 3.0.0-alpha.3 vs platform at 2.4.0-beta.20), independent version drift, dynamic import indirection for lint compliance, and a confusing placement of build-time utilities inside a runtime router package — all without a consumer that justifies it.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
apps/blog-app/src/routeTree.gen.ts (1)

325-340: ⚠️ Potential issue | 🟠 Major

Regenerate this route tree; the lib content path is still malformed.

Lines 326-328 still collapse "/libs/shared/feature/src/content/test.md" into "/libs/shared/featuretest", and Line 417 repeats that bad token in the lookup map. That will surface the wrong full path in typed navigation/runtime lookups. Please rerun codegen; if it reproduces, the src/content normalization is still dropping the separator and needs to emit .../feature/test instead.

Also applies to: 417-417

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/blog-app/src/routeTree.gen.ts` around lines 325 - 340, The route tree
generation collapsed the path segment separator so the route id and fullPath
show "/libs/shared/featuretest" instead of the correct
"/libs/shared/feature/test" (see the generated entry that currently has id
"/libs/shared/featuretest" and sourceFile
"/libs/shared/feature/src/content/test.md" and the lookup map entry for the same
token). Regenerate the route tree codegen and fix the path normalization logic
that builds route ids/fullPath from sourceFile (the normalization that converts
"src/content/test.md" into the route segment), ensuring it preserves the
separator so it emits ".../feature/test" (and update any code that fills
AnalogFileRoutesById keys/lookups to use the corrected token).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@apps/blog-app/src/routeTree.gen.ts`:
- Around line 325-340: The route tree generation collapsed the path segment
separator so the route id and fullPath show "/libs/shared/featuretest" instead
of the correct "/libs/shared/feature/test" (see the generated entry that
currently has id "/libs/shared/featuretest" and sourceFile
"/libs/shared/feature/src/content/test.md" and the lookup map entry for the same
token). Regenerate the route tree codegen and fix the path normalization logic
that builds route ids/fullPath from sourceFile (the normalization that converts
"src/content/test.md" into the route segment), ensuring it preserves the
separator so it emits ".../feature/test" (and update any code that fills
AnalogFileRoutesById keys/lookups to use the corrected token).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6dd7cd78-d3b7-40f1-802d-6518f2b5a497

📥 Commits

Reviewing files that changed from the base of the PR and between 4aec881 and 3352f98.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml and included by none
📒 Files selected for processing (4)
  • apps/analog-app/src/routeTree.gen.ts
  • apps/blog-app/src/main.ts
  • apps/blog-app/src/routeTree.gen.ts
  • package.json
✅ Files skipped from review due to trivial changes (2)
  • apps/blog-app/src/main.ts
  • apps/analog-app/src/routeTree.gen.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • package.json

@github-actions github-actions Bot added the scope:vite-plugin-nitro Changes in @analogjs/vite-plugin-nitro label Mar 24, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/vite-plugin-nitro/src/lib/build-sitemap.ts (1)

196-200: Verify: new URL(host) throws on invalid host strings.

normalizeSitemapHost uses new URL(host) which throws TypeError for invalid URLs. This provides early validation but means consumers with invalid host values will see an uncaught error.

This is likely intentional (fail-fast), but worth noting that the error won't be user-friendly. Consider whether a more descriptive error message would help.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/vite-plugin-nitro/src/lib/build-sitemap.ts` around lines 196 - 200,
normalizeSitemapHost currently calls new URL(host) which will throw a raw
TypeError on invalid input; wrap the URL construction in a try/catch inside
normalizeSitemapHost (or pre-validate the string) and rethrow a clearer,
user-friendly error that includes the problematic host string and guidance
(e.g., "Invalid sitemap host: '<host>' — expected a fully qualified URL
including scheme"), or return a controlled fallback; reference
normalizeSitemapHost and the URL construction to locate where to add the
try/catch and the improved error message.
packages/vite-plugin-nitro/src/lib/build-sitemap.spec.ts (1)

16-21: Minor: Reset order in afterEach may not work as intended.

vi.restoreAllMocks() restores mocks to their original implementation, which would undo the mockReturnValue(true) set on line 18. The subsequent mockReset() calls then clear call history but leave the mock in its restored state.

If the intent is to have existsSyncMock return true by default for subsequent tests, consider reordering or using beforeEach for the default setup.

That said, since each test explicitly sets existsSyncMock.mockReturnValue(...) when needed, this is functionally correct — just slightly confusing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/vite-plugin-nitro/src/lib/build-sitemap.spec.ts` around lines 16 -
21, The afterEach resets/ restores mocks in the wrong order causing
existsSyncMock.mockReturnValue(true) to be undone by vi.restoreAllMocks(); fix
by either moving the default setup into a beforeEach (set
existsSyncMock.mockReturnValue(true) there) or by reordering afterEach so
vi.restoreAllMocks() runs first and then call
existsSyncMock.mockReturnValue(true), keeping mkdirSyncMock.mockReset() and
writeFileSyncMock.mockReset() as needed; refer to the afterEach block and the
mocks existsSyncMock, mkdirSyncMock, writeFileSyncMock and the call
vi.restoreAllMocks().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/vite-plugin-nitro/src/lib/build-sitemap.spec.ts`:
- Around line 16-21: The afterEach resets/ restores mocks in the wrong order
causing existsSyncMock.mockReturnValue(true) to be undone by
vi.restoreAllMocks(); fix by either moving the default setup into a beforeEach
(set existsSyncMock.mockReturnValue(true) there) or by reordering afterEach so
vi.restoreAllMocks() runs first and then call
existsSyncMock.mockReturnValue(true), keeping mkdirSyncMock.mockReset() and
writeFileSyncMock.mockReset() as needed; refer to the afterEach block and the
mocks existsSyncMock, mkdirSyncMock, writeFileSyncMock and the call
vi.restoreAllMocks().

In `@packages/vite-plugin-nitro/src/lib/build-sitemap.ts`:
- Around line 196-200: normalizeSitemapHost currently calls new URL(host) which
will throw a raw TypeError on invalid input; wrap the URL construction in a
try/catch inside normalizeSitemapHost (or pre-validate the string) and rethrow a
clearer, user-friendly error that includes the problematic host string and
guidance (e.g., "Invalid sitemap host: '<host>' — expected a fully qualified URL
including scheme"), or return a controlled fallback; reference
normalizeSitemapHost and the URL construction to locate where to add the
try/catch and the improved error message.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9d5aedc4-62a3-462b-9531-400d4bdc954c

📥 Commits

Reviewing files that changed from the base of the PR and between 3352f98 and ca0b250.

📒 Files selected for processing (10)
  • apps/docs-app/docs/features/routing/typed-routes.md
  • apps/docs-app/docs/features/server/static-site-generation.md
  • packages/platform/src/index.ts
  • packages/platform/src/lib/options.ts
  • packages/vite-plugin-nitro/src/index.ts
  • packages/vite-plugin-nitro/src/lib/build-sitemap.spec.ts
  • packages/vite-plugin-nitro/src/lib/build-sitemap.ts
  • packages/vite-plugin-nitro/src/lib/options.ts
  • packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.spec.ts
  • packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/docs-app/docs/features/routing/typed-routes.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/platform/src/index.ts
  • packages/platform/src/lib/options.ts

@benpsnyder

benpsnyder commented Mar 24, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai quick update on the latest push:

  • Added support for the single-extras navigate form so injectNavigate() now handles navigate('/about', { replaceUrl: true }) cleanly.
  • Expanded sitemap work in vite-plugin-nitro, including customizable sitemap generation behavior and dynamic client output directory resolution.
  • Pulled some plugin cleanup into a separate closeBundle helper to keep the Nitro flow easier to follow.
  • Included a small typed-routes docs clarity pass alongside the code changes.

The newest commits on the branch are ca0b250, 15712c9, 607d2b1, and 0caf1f6.

@coderabbitai

coderabbitai Bot commented Mar 24, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Fetch the details of the 4 commits
gh api repos/analogjs/analog/commits/ca0b250 --jq '.commit.message' 2>/dev/null || echo "ca0b250 not found"
gh api repos/analogjs/analog/commits/15712c9 --jq '.commit.message' 2>/dev/null || echo "15712c9 not found"
gh api repos/analogjs/analog/commits/607d2b1 --jq '.commit.message' 2>/dev/null || echo "607d2b1 not found"
gh api repos/analogjs/analog/commits/0caf1f6 --jq '.commit.message' 2>/dev/null || echo "0caf1f6 not found"

Length of output: 282


🏁 Script executed:

#!/bin/bash
# Fetch the diffs for all 4 commits
echo "=== COMMIT ca0b250 ==="
gh api repos/analogjs/analog/commits/ca0b250 --jq '.files[] | "FILE: \(.filename)\nADDITIONS: \(.additions)\nDELETIONS: \(.deletions)\nPATCH:\n\(.patch // "(binary or too large)")\n---"' 2>/dev/null

echo "=== COMMIT 15712c9 ==="
gh api repos/analogjs/analog/commits/15712c9 --jq '.files[] | "FILE: \(.filename)\nADDITIONS: \(.additions)\nDELETIONS: \(.deletions)\nPATCH:\n\(.patch // "(binary or too large)")\n---"' 2>/dev/null

echo "=== COMMIT 607d2b1 ==="
gh api repos/analogjs/analog/commits/607d2b1 --jq '.files[] | "FILE: \(.filename)\nADDITIONS: \(.additions)\nDELETIONS: \(.deletions)\nPATCH:\n\(.patch // "(binary or too large)")\n---"' 2>/dev/null

echo "=== COMMIT 0caf1f6 ==="
gh api repos/analogjs/analog/commits/0caf1f6 --jq '.files[] | "FILE: \(.filename)\nADDITIONS: \(.additions)\nDELETIONS: \(.deletions)\nPATCH:\n\(.patch // "(binary or too large)")\n---"' 2>/dev/null

Length of output: 62524


@benpsnyder thanks for the heads‑up — I pulled the 4 commits and did a focused pass. Highlights and confirmations:

Inject navigate, typing, and runtime guards

  • 0caf1f6
    • inject-navigate.ts now supports both:
      • navigate(path, options?, extras?) and
      • navigate(path, extras) for the single-extras form, with runtime disambiguation via isRoutePathOptionsBase. Typing is tightened with NavigateWithExtrasArgs so “extras-only” is allowed for routes with optional options, but not when params are required. Verified by the new spec that exercises the extras-only call for '/about'. Files:
      • packages/router/src/lib/inject-navigate.ts (+overload +disambiguation)
      • packages/router/src/lib/inject-navigate.spec.ts (adds the extras-only case)
    • route-path typing now infers whether params are required or entirely optional:
      • Optional-catch-all routes make options optional; added compile-time coverage in route-path.generated.typetest.spec.ts.
      • Runtime now throws if a required catch‑all gets an empty array (buildUrl test added).
      • Files: packages/router/src/lib/route-path.ts, route-path.spec.ts, route-path.generated.typetest.spec.ts
    • inject-typed-params now uses extractRouteParams and skips warnings on optional catch‑all omission; test added.

Typed routes docs and sitemap docs

  • ca0b250
    • apps/docs-app/docs/features/routing/typed-routes.md: added Supported Route Patterns, Multi‑Library Route Composition, Route Import Auto‑Injection, and CI Staleness Verification — this lines up with the plugin behavior we already validated (auto import and freshness checks).
    • apps/docs-app/docs/features/server/static-site-generation.md: updated to the new sitemap model (defaults/include/exclude/transform), clarified Nitro publicDir target, and “omit lastmod if not provided.”

Nitro: dynamic client outDir and sitemap pipeline

  • 15712c9

    • Dynamic client output path uses getEnvironmentBuildOutDir + resolveBuiltClientOutputPath so builder.environments.client config wins over stale paths. Tests verify that the Nitro alias/publicAssets and #analog/index pick up the finalized client outDir.
    • Files: packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts (+helpers), vite-plugin-nitro.spec.ts (finalized client env test).
  • ca0b250 (sitemap refactor)

    • build-sitemap.ts reworked around:
      • Route collection (prerender routes + include), per‑entry resolve with defaults + route‑specific overrides, transform hook, exclude (glob/RegExp/function), internal route filtering via apiPrefix, and safe output (mkdir -p, refuse CWD).
      • Writes optional lastmod/changefreq/priority only when present; host normalization handles trailing slashes; URL building encodes spaces as expected.
    • Tests cover no routes, trailing slash host + metadata, include/defaults/exclude/transform, output‑dir creation, and refusing CWD.
    • vite-plugin-nitro.ts now:
      • Tracks canonical sitemapRoutes, passes Nitro publicDir via getNitroPublicOutputDir, and supplies { apiPrefix } to buildSitemap in both builder.buildApp and closeBundle paths.
    • Type surface expanded and re-exported through platform and vite-plugin-nitro index.ts so consumers get Sitemap* types from @analogjs/platform.

closeBundle extraction

  • 607d2b1
    • Tests now use a runCloseBundle helper to exercise both function and object.handler forms — keeps the spec resilient to Vite plugin API shapes.

Typed-routes plugin polish

  • Entry-import auto-injection is skipped during verify/verifyOnBuild (so we don’t mutate app entry files in those modes); tests assert no mutation.
  • JSON‑LD manifest inputs are filtered to the canonical manifest winners (aligning with collision precedence); test added for that case.
  • route-file-discovery accepts already-normalized additional dirs without double-prefixing workspaceRoot; spec added.

Light suggestions (non-blocking)

  • Type tests: consider a negative compile-time assertion (// @ts-expect-error) that navigate('/users/[id]', { replaceUrl: true }) fails when params are required, to lock in the overload intent alongside the positive test you added for '/about'.
  • Sitemap tests: you already cover space encoding in path segments; adding one case for querystring encoding (e.g., '/search?q=hello world') would round out the URL-encoding coverage.

Everything else looks solid. Nice work tightening the contracts, guarding verify/freshness paths against unintended mutations, and expanding doc clarity exactly where devs will look.


🧠 Learnings used
Learnt from: benpsnyder
Repo: analogjs/analog PR: 2144
File: packages/content-plugin/tsdown.config.ts:24-24
Timestamp: 2026-03-22T22:10:37.534Z
Learning: In the analogjs/analog monorepo, `tsdown` (and Vite lib) build configs intentionally write their `outDir` directly into `node_modules/analogjs/<package>` (not `dist/`). This is the monorepo-wide convention that enables Nx caching and local dev package resolution without a separate `npm link` step. Do not flag this pattern as an antipattern or suggest routing output to `dist/` instead.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2125
File: packages/router/src/lib/route-path.typetest.spec.ts:62-155
Timestamp: 2026-03-24T13:22:47.567Z
Learning: In `packages/router/src/lib/route-path.typetest.spec.ts`, the `MockRouteTable` interface intentionally does NOT augment `AnalogRouteTable`. The file header states that since `AnalogRouteTable` is empty by default (populated only by generated code), type utility correctness is validated by testing conditional types directly against `MockRouteTable`. Do not flag this as a missing augmentation or suggest adding `declare module '...' { interface AnalogRouteTable extends MockRouteTable {} }`.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2144
File: packages/vitest-angular-tools/vite.config.lib.ts:30-34
Timestamp: 2026-03-22T23:32:34.648Z
Learning: In `packages/vitest-angular-tools/vite.config.lib.ts`, `build.emptyOutDir` is intentionally set to `false`. The output directory is `node_modules/analogjs/vitest-angular/src/lib/tools`, which is a subdirectory of the `vitest-angular` package output. Setting `emptyOutDir: true` would delete sibling files already written by the parent `vitest-angular` build (which runs immediately before `nx build vitest-angular-tools`). Do not flag this as a bug. The same pattern applies to `packages/vite-plugin-angular-tools/vite.config.lib.ts`.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2144
File: packages/vite-plugin-angular-tools/vite.config.lib.ts:10-11
Timestamp: 2026-03-22T22:11:42.083Z
Learning: In `packages/vite-plugin-angular-tools/vite.config.lib.ts` (and other `vite.config.lib.ts` build configs in analogjs/analog), the import of `oxcDtsPlugin` from `../../tools/build/shared-plugins.ts` uses a relative cross-package path with an `// eslint-disable-next-line nx/enforce-module-boundaries` suppression. This is intentional and correct: `tools/build/shared-plugins.ts` is a build-time utility with no registered TypeScript path alias in `tsconfig.base.json`. Do not flag this relative import or its eslint suppression as a violation — suggesting a non-existent workspace alias would break the import.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2144
File: packages/vite-plugin-angular-tools/vite.config.lib.ts:30-34
Timestamp: 2026-03-22T22:06:57.824Z
Learning: In `packages/vite-plugin-angular-tools/vite.config.lib.ts`, `build.emptyOutDir` is intentionally set to `false`. The output directory is `node_modules/analogjs/vite-plugin-angular/src/lib/tools`, which is a subdirectory of the `vite-plugin-angular` package output. Setting `emptyOutDir: true` would delete sibling files already written by the parent `vite-plugin-angular` build, breaking the combined build pipeline. Do not flag this as a bug.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2125
File: packages/router/manifest/src/index.ts:161-165
Timestamp: 2026-03-24T13:22:26.237Z
Learning: In `packages/router/manifest/src/index.ts`, `detectSchemaExports` intentionally uses simple line-start regexes (`/^\s*export\s+const\s+routeParamsSchema\b/m`) without stripping comments first. This is a known v1 limitation: a commented-out `// export const routeParamsSchema` will produce a false positive. The trade-off is explicitly accepted and documented in the test at `packages/router/src/lib/route-manifest.spec.ts` around line 723 ("Simple regex doesn't filter comments — acceptable for v1."). Do not flag this as a bug or propose AST-based detection; the fix is deferred to a future version.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2125
File: packages/vite-plugin-routes/src/lib/route-file-discovery.ts:53-83
Timestamp: 2026-03-24T13:22:57.659Z
Learning: In `packages/vite-plugin-routes/src/lib/route-file-discovery.ts`, `getDiscoveredFileKind` intentionally uses convention-based `includes()` checks (e.g. `normalized.includes('/src/app/pages/')`, `normalized.includes('/src/app/routes/')`, `normalized.includes('/src/content/')`) to classify standard/well-known dirs. This is acceptable because the dev watcher delivers absolute paths and those path segments are sufficiently specific. Configurable `additionalPagesDirs`/`additionalContentDirs` are already root-scoped via `isWithinDir()`. Do not flag the standard-dir `includes()` checks as a monorepo scope-leak risk — the design is intentional.

Learnt from: CR
Repo: analogjs/analog PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-23T01:24:01.777Z
Learning: Applies to packages/nx-plugin/**/*.{ts,tsx} : Cross compatibility with Nx is strongly encouraged; prefer schematics and builders for Analog first-party solutions

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2144
File: packages/content-plugin/tsdown.config.ts:24-24
Timestamp: 2026-03-22T22:10:37.534Z
Learning: In `packages/content-plugin/tsdown.config.ts` (and similar CJS sub-package build configs in analogjs/analog), the `build:done` hook writes a minimal `package.json` containing only `{ "type": "commonjs" }` into the output directory. This is intentional: the workspace root has `"type": "module"`, so each CJS output directory needs this file to signal CommonJS to Node.js. The full package.json with `name`, `version`, `main`, `exports`, and all other metadata is copied into the output during the publish step, not the dev build. Do not flag this minimal package.json as stripping required metadata.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2144
File: packages/platform/vite.config.lib.ts:0-0
Timestamp: 2026-03-22T23:12:26.945Z
Learning: In `tools/build/shared-plugins.ts` (and any package that uses it), the `oxcDtsPlugin` catch blocks around `isolatedDeclarationSync` calls were updated (PR `#2144`) to re-throw with file context on genuine failures. The normal "best-effort skip" path (file lacks explicit type annotations) is NOT an exception — `isolatedDeclarationSync` simply returns empty `code`, so the catch block change does not affect that case. Only the "already exists" probe catch around the secondary walk remains intentionally silent. Do not revert the catch blocks back to silent/empty form for the declaration-emission loops.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2125
File: packages/router/src/lib/route-manifest.spec.ts:723-734
Timestamp: 2026-03-24T13:22:38.164Z
Learning: In `packages/router/src/lib/route-manifest.spec.ts` (and the corresponding `detectSchemaExports` implementation in `packages/router/manifest/src/index.ts`), the regex-based schema export detector intentionally false-positives on commented-out `export const routeParamsSchema`/`export const routeQuerySchema` lines. This is an accepted v1 limitation explicitly documented in the test. The test expectation `expect(result.hasParamsSchema).toBe(true)` for commented-out content is correct and intentional. Do not flag this as a bug or suggest changing the expectation to `false` — it will be addressed if/when schema detection is upgraded to AST-based parsing.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2139
File: packages/platform/src/lib/router-plugin.ts:32-42
Timestamp: 2026-03-23T01:53:21.778Z
Learning: In `packages/platform/src/lib/router-plugin.ts`, `normalizeWatchedDir` (used to build `additionalPagesDirs`, `additionalContentDirs`, and the root-relative entries in `getRouteLikeDirs`) uses a three-way dispatch: (1) paths that already start with `${workspaceRoot}/` or equal `workspaceRoot` are used directly (already fully resolved); (2) paths starting with `/` that are NOT already under workspaceRoot are prefixed with `workspaceRoot` (workspace-relative convention, e.g. `/libs/shared/feature/src/pages`); (3) bare relative paths use `resolve(workspaceRoot, dir)`. Trailing slashes are stripped so `startsWith(`${dir}/`)` comparisons work consistently. This mirrors the `normalizeContentDir` logic in `content-plugin.ts`.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2141
File: packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts:467-471
Timestamp: 2026-03-23T01:45:47.267Z
Learning: In `packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts`, `nitroConfig.alias = {}` (around the alias-initialization block) is a safe initialization, NOT a destructive overwrite. `nitroConfig` is built from scratch with no `alias` property, so the `= {}` sets a clean baseline. User-provided `nitroOptions.alias` values are merged in later via `mergeConfig(nitroConfig, nitroOptions)`, and `applySsrEntryAlias` further spreads existing aliases before adding `#analog/ssr`. Do not flag this pattern as silently dropping user aliases.

Learnt from: benpsnyder
Repo: analogjs/analog PR: 2138
File: packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts:351-354
Timestamp: 2026-03-23T17:08:15.533Z
Learning: In `packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts`, `resolveClientOutputPath` resolves the built client output directory using these rules: (1) if `cachedPath` is set, return it directly; (2) if `configuredOutDir` is explicitly provided by the user, resolve it as `resolve(workspaceRoot, rootDir, configuredOutDir)` (relative to the app root); (3) otherwise (no explicit `build.outDir`), always use `resolve(workspaceRoot, 'dist', rootDir, 'client')` — matching the environment build config default. The `ssrBuild` parameter was removed after unifying both branches. This convention is required for `registerIndexHtmlVirtual()` and `publicAssets` to read from the correct directory in nested monorepo setups (e.g., `apps/my-app`).

@github-actions github-actions Bot added the scope:astro-angular Changes in @analogjs/astro-angular label Mar 24, 2026
@github-actions github-actions Bot removed scope:astro-angular Changes in @analogjs/astro-angular scope:content Changes in @analogjs/content scope:content-plugin Changes in @analogjs/content-plugin scope:create-analog Changes in create-analog scope:nx-plugin Changes in @analogjs/nx-plugin scope:platform Changes in @analogjs/platform scope:storybook-angular Changes in @analogjs/storybook-angular scope:vite-plugin-nitro Changes in @analogjs/vite-plugin-nitro scope:ci GitHub workflow changes labels Mar 25, 2026
@benpsnyder benpsnyder force-pushed the feat/file-based-routing-type-safe branch from b163d05 to f15da7d Compare March 25, 2026 01:57
@github-actions github-actions Bot removed the scope:vitest-angular Changes in @analogjs/vitest-angular label Mar 25, 2026
@benpsnyder

benpsnyder commented Mar 25, 2026

Copy link
Copy Markdown
Contributor Author

@brandonroberts ready to merge ... finally.
thus concludes the v3 feature run! time for some hardcore stabilization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope:docs Documentation changes scope:repo Repository metadata and tooling scope:router Changes in @analogjs/router scope:trpc Changes in @analogjs/trpc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants