refactor: overhaul stylesheet pipeline and improve Tailwind HMR across Analog#2226
refactor: overhaul stylesheet pipeline and improve Tailwind HMR across Analog#2226brandonroberts merged 33 commits intoanalogjs:alphafrom
Conversation
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds Angular HMR ( Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes |
|
This PR touches multiple package scopes: Please confirm the changes are closely related. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
packages/router/src/lib/routes.spec.ts (1)
978-1012: Spy cleanup should useafterEachfor robustness.If an assertion fails before
spy.mockRestore(), the spy won't be cleaned up, potentially affecting subsequent tests. Other describe blocks in this file (lines 874-876, 1058-1060) useafterEachfor spy restoration.♻️ Suggested pattern for consistent cleanup
describe('duplicate route precedence', () => { class AppRouteComponent {} class SharedRouteComponent {} + + afterEach(() => { + vi.restoreAllMocks(); + }); it('prefers app-local page routes over additional/shared page routes', async () => { const spy = vi.spyOn(console, 'warn').mockImplementation(() => { /* noop */ }); // ... test body ... - - spy.mockRestore(); }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router/src/lib/routes.spec.ts` around lines 978 - 1012, The test creates a console.warn spy via vi.spyOn inside the it block and calls spy.mockRestore() at the end, which risks leaking the spy if the assertion fails; refactor the test to create the spy in a beforeEach (or at top of the describe) and restore it in an afterEach so cleanup always runs; update the 'duplicate route precedence' describe to set up the spy for console.warn (used in the test that calls createBaseRoutes and inspects route.loadChildren) in beforeEach and call spy.mockRestore() in afterEach to ensure robust teardown.apps/analog-app/vite.config.ts (1)
78-78: Consider migrating fromliveReloadtohmrfor consistency.This PR introduces
hmras the primary option withliveReloadretained as a deprecated compatibility alias. The example app should demonstrate the preferred approach.♻️ Suggested change
- liveReload: true, + hmr: true,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/analog-app/vite.config.ts` at line 78, Replace the dev-server option usage of liveReload with the preferred hmr option: add an hmr entry (e.g. hmr: true or an hmr config object) and keep liveReload present only as a deprecated compatibility alias (leave liveReload: true but mark/comment it deprecated or move it alongside hmr). Update the dev server config where liveReload is currently set (the liveReload property in vite.config.ts) to use hmr as the primary key and retain liveReload solely for backward compatibility.packages/vite-plugin-angular/src/lib/utils/debug.ts (1)
4-7: Unused imports:DEBUG_LOG_FILENAMEandwrapInstancesForFileLogThese symbols are imported but not referenced anywhere in this module. The file-logging wrappers are applied inside
debug-harness.tsviainstallFileWrappers, which receives theinstanceGroupsfrom the harness config—this file only passes the instances, it doesn't call the wrappers directly.If these were added for future use, consider deferring the import until needed to keep the module surface clean.
🧹 Remove unused imports
-import { - DEBUG_LOG_FILENAME, - wrapInstancesForFileLog, -} from './debug-log-file.js';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/utils/debug.ts` around lines 4 - 7, Remove the unused imports DEBUG_LOG_FILENAME and wrapInstancesForFileLog from the import statement in debug.ts since this module doesn't reference them; if you intended to use them later, instead perform a dynamic import (e.g., import('./debug-log-file.js') inside the function that needs them) so the module surface stays clean and avoids importing file-logging helpers that are applied elsewhere (installFileWrappers in debug-harness).packages/vite-plugin-angular/src/lib/stylesheet-registry.ts (2)
110-121: Basename alias can cause lookup collisions, but impact is limited to dev-time HMR.When multiple components have stylesheets with the same basename (e.g.,
libs/feature-a/card.component.cssandlibs/feature-b/card.component.css), the later registration overwrites the basename alias. This means a basename-only lookup would serve the most recent content.This is acceptable because:
- The
publicId(sha256 hash) remains unique and is the primary lookup key- Production builds inline styles and don't use this registry
- In HMR scenarios, the most recent edit is typically what we want
If strict correctness becomes necessary, the basename alias could be scoped or omitted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/stylesheet-registry.ts` around lines 110 - 121, The current registry pushes basename(resourceFile) into the aliases array which can cause lookup collisions when different components share the same stylesheet basename; update the registration in stylesheet-registry.ts (the aliases array construction around resourceFile/normalizedResourceFile and basename) to avoid collisions by either removing the basename alias entirely or scoping it (e.g., prepend a component-specific prefix or part of the normalized path) so that basename no longer overwrites unrelated entries during HMR; ensure the chosen change still includes resourceFile and normalizedResourceFile entries and document the decision in a comment near the aliases construction.
70-82:rewriteRelativeCssImportshandles common cases, buturl()syntax is unhandled.The regex
/@import\s+(['"])(\.[^'"]+)\1/gcorrectly rewrites string-based relative imports but doesn't handle the@import url(./foo.css)syntax. This is likely acceptable since:
- Angular component stylesheets rarely use
url()for imports- The primary use case is Tailwind's
@referencewhich uses string syntaxIf
url()support becomes necessary, the regex could be extended:🔧 Optional: Extend to handle url() syntax
export function rewriteRelativeCssImports( code: string, filename: string, ): string { const cssDir = dirname(filename); - return code.replace( + // Handle string-based `@import` + let result = code.replace( /@import\s+(['"])(\.[^'"]+)\1/g, (_match, quote, relPath) => { const absPath = resolve(cssDir, relPath); return `@import ${quote}${absPath}${quote}`; }, ); + // Handle url() based `@import` + result = result.replace( + /@import\s+url\((['"]?)(\.[^'")\s]+)\1\)/g, + (_match, quote, relPath) => { + const absPath = resolve(cssDir, relPath); + return `@import url(${quote}${absPath}${quote})`; + }, + ); + return result; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/stylesheet-registry.ts` around lines 70 - 82, rewriteRelativeCssImports only rewrites string-form `@import` ('./x.css') but misses `@import` url(./x.css) and `@import` url('./x.css') forms; update the regex in rewriteRelativeCssImports to also match url(...) variants (both quoted and unquoted) and in the replace callback detect whether the match used url(...) or a plain string so you can resolve the relative relPath with resolve(cssDir, relPath) and recompose the import preserving the original wrapper (either `@import` "..." / `@import` '...' or `@import` url(...) with its quotes/unquoted content) to avoid breaking existing import syntax.
🤖 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/routes.spec.ts`:
- Around line 994-1003: The test incorrectly looks up a nested route by matching
'blog/:slug' on the top-level routes array; update the test to traverse the
nested route tree returned by createBaseRoutes to find the child route (first
find the top-level route with path 'blog', then access its children and find the
child with path ':slug' and call its loadChildren) so route is defined and
loadedRoutes can be asserted against AppRouteComponent (update references in
this spec around the routes variable and route.loadChildren usage).
In `@packages/vite-plugin-angular/src/lib/angular-jit-plugin.ts`:
- Around line 48-51: The catch block that currently only calls debugStyles('jit
css compilation error', { styleIdHash, error: String(e) }) must also emit a
visible dev warning/error (e.g., console.warn or the plugin/vite logger) so JIT
preprocessing failures are not silent; update the error path in
angular-jit-plugin.ts to both call debugStyles for structured details and also
emit a human-readable warning including styleIdHash and the error message so
users see the failure in normal dev output, while still returning the existing
empty exported stylesheet fallback.
In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts`:
- Around line 28-33: The current test "accepts liveReload as a compatibility
alias" calls angular({ liveReload: true }) but only asserts for a plugin that is
always present; change it to assert that HMR-specific plugins returned by
angularVitestPlugins() (i.e., the plugins that should be included when HMR is
enabled) are present when calling angular({ liveReload: true }) and are absent
when calling angular({ liveReload: false }) or default, by comparing the plugin
names from plugins.map(p => p.name) against the names produced by
angularVitestPlugins() (or specific HMR plugin name strings) to verify
liveReload actually toggles HMR inclusion.
In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.ts`:
- Around line 558-574: When Angular version is detected as angularFullVersion <
190000 we silently disable HMR by setting pluginOptions.hmr = false; add a
user-facing warning there so users know their hmr config was overridden. Update
the branch that checks angularFullVersion to call a visible logger (e.g.,
console.warn or the plugin's logger) with a clear message that Angular <19 does
not support externalRuntimeStyles/_enableHmr and HMR is disabled, and mirror the
debugHmr context (angularVersion and isTest where relevant) so debugging info is
available; keep the existing isTest block and debugHmr call unchanged.
In `@packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts`:
- Line 35: The import debugPrerender is unused; either remove it from the import
list in vite-plugin-nitro (alongside debugNitro and debugSsr) or add explicit
prerender debug calls where prerender route handling happens (the prerender
route handling blocks) using debugPrerender(...) to log prerender-specific
events; update references in the file so only used debug instances are imported
and ensure any added debugPrerender calls mirror the style/arguments of existing
debugNitro/debugSsr invocations.
---
Nitpick comments:
In `@apps/analog-app/vite.config.ts`:
- Line 78: Replace the dev-server option usage of liveReload with the preferred
hmr option: add an hmr entry (e.g. hmr: true or an hmr config object) and keep
liveReload present only as a deprecated compatibility alias (leave liveReload:
true but mark/comment it deprecated or move it alongside hmr). Update the dev
server config where liveReload is currently set (the liveReload property in
vite.config.ts) to use hmr as the primary key and retain liveReload solely for
backward compatibility.
In `@packages/router/src/lib/routes.spec.ts`:
- Around line 978-1012: The test creates a console.warn spy via vi.spyOn inside
the it block and calls spy.mockRestore() at the end, which risks leaking the spy
if the assertion fails; refactor the test to create the spy in a beforeEach (or
at top of the describe) and restore it in an afterEach so cleanup always runs;
update the 'duplicate route precedence' describe to set up the spy for
console.warn (used in the test that calls createBaseRoutes and inspects
route.loadChildren) in beforeEach and call spy.mockRestore() in afterEach to
ensure robust teardown.
In `@packages/vite-plugin-angular/src/lib/stylesheet-registry.ts`:
- Around line 110-121: The current registry pushes basename(resourceFile) into
the aliases array which can cause lookup collisions when different components
share the same stylesheet basename; update the registration in
stylesheet-registry.ts (the aliases array construction around
resourceFile/normalizedResourceFile and basename) to avoid collisions by either
removing the basename alias entirely or scoping it (e.g., prepend a
component-specific prefix or part of the normalized path) so that basename no
longer overwrites unrelated entries during HMR; ensure the chosen change still
includes resourceFile and normalizedResourceFile entries and document the
decision in a comment near the aliases construction.
- Around line 70-82: rewriteRelativeCssImports only rewrites string-form `@import`
('./x.css') but misses `@import` url(./x.css) and `@import` url('./x.css') forms;
update the regex in rewriteRelativeCssImports to also match url(...) variants
(both quoted and unquoted) and in the replace callback detect whether the match
used url(...) or a plain string so you can resolve the relative relPath with
resolve(cssDir, relPath) and recompose the import preserving the original
wrapper (either `@import` "..." / `@import` '...' or `@import` url(...) with its
quotes/unquoted content) to avoid breaking existing import syntax.
In `@packages/vite-plugin-angular/src/lib/utils/debug.ts`:
- Around line 4-7: Remove the unused imports DEBUG_LOG_FILENAME and
wrapInstancesForFileLog from the import statement in debug.ts since this module
doesn't reference them; if you intended to use them later, instead perform a
dynamic import (e.g., import('./debug-log-file.js') inside the function that
needs them) so the module surface stays clean and avoids importing file-logging
helpers that are applied elsewhere (installFileWrappers in debug-harness).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7c843a0a-ace2-4a12-896a-d376f1f711e9
⛔ Files ignored due to path filters (3)
.gitignoreis excluded by none and included by nonePlan.mdis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (35)
apps/analog-app/package.jsonapps/analog-app/src/app/app.config.tsapps/analog-app/vite.config.tsapps/blog-app/src/app/app.config.tspackage.jsonpackages/create-analog/__tests__/cli.spec.tspackages/create-analog/index.jspackages/nx-plugin/src/generators/app/generator.spec.tspackages/nx-plugin/src/generators/app/lib/add-tailwind-config.tspackages/nx-plugin/src/generators/app/lib/add-tailwind-helpers.tspackages/nx-plugin/src/generators/app/versions/nx_18_X/versions.tspackages/nx-plugin/src/generators/app/versions/tailwind-dependencies.tspackages/platform/src/lib/options.tspackages/platform/src/lib/platform-plugin.tspackages/platform/src/lib/utils/debug.spec.tspackages/router/src/lib/route-builder.tspackages/router/src/lib/routes.spec.tspackages/storybook-angular/README.mdpackages/storybook-angular/src/lib/preset.spec.tspackages/storybook-angular/src/lib/preset.tspackages/storybook-angular/src/types.tspackages/vite-plugin-angular/src/lib/angular-jit-plugin.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin-live-reload.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.tspackages/vite-plugin-angular/src/lib/host.spec.tspackages/vite-plugin-angular/src/lib/host.tspackages/vite-plugin-angular/src/lib/plugins/file-replacements.plugin.tspackages/vite-plugin-angular/src/lib/stylesheet-registry.spec.tspackages/vite-plugin-angular/src/lib/stylesheet-registry.tspackages/vite-plugin-angular/src/lib/utils/debug.spec.tspackages/vite-plugin-angular/src/lib/utils/debug.tspackages/vite-plugin-nitro/src/index.tspackages/vite-plugin-nitro/src/lib/vite-plugin-nitro.tstools/scripts/debug-nx-project-graph.mts
packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/analog-app-e2e/tests/legacy-server-action.spec.ts`:
- Line 6: The test is waiting on a non-existent attribute selector (page.locator
with selector "form[data-state]") which causes beforeEach timeouts; update the
readiness check in the test to wait for a stable existing element instead—either
change the locator to target the plain "form" element (using
page.locator("form").waitFor) or add and target a stable attribute (e.g.,
data-testid on the form) that you also add to the legacy-action page component;
update the test's beforeEach locator reference (page.locator and the selector
"form[data-state]") to use the chosen stable selector.
In `@apps/analog-app/src/app/pages/legacy-action.page.ts`:
- Around line 27-28: The label's for attribute ("email") isn't linked to the
input because the input lacks id="email"; update the input element in
legacy-action.page.ts (the <input type="email" name="email" />) to include
id="email" (or change the label's for to match the existing id if you prefer a
different identifier) so the <label for="email"> correctly associates with the
input control for accessibility and form behavior.
In `@apps/docs-app/docs/integrations/angular-material/index.md`:
- Around line 167-170: The manual install command only adds `@tailwindcss/vite`
but the note explains using Tailwind v4 with a PostCSS pipeline; update the
install step where the docs add "@tailwindcss/vite" to instead install the full
set of dev dependencies required for the described flow: tailwindcss,
`@tailwindcss/vite`, postcss, and `@tailwindcss/postcss` (use -D/dev flag). Locate
the npm install line in the Angular Material integration docs and replace the
single-package install with the multi-package dev-install so the described setup
matches the actual packages required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 02f76b7d-eaa5-4233-9b4b-103f899e7565
📒 Files selected for processing (17)
apps/analog-app-e2e/tests/legacy-server-action.spec.tsapps/analog-app/src/app/pages/client/(client).page.tsapps/analog-app/src/app/pages/legacy-action.page.tsapps/analog-app/src/app/pages/legacy-action.server.tsapps/analog-app/src/app/pages/newsletter.page.tsapps/analog-app/src/app/pages/newsletter.server.tsapps/analog-app/src/routeTree.gen.tsapps/docs-app/docs/guides/migrating.mdapps/docs-app/docs/integrations/angular-material/index.mdapps/docs-app/docs/integrations/storybook/index.mdapps/docs-app/docs/packages/create-analog/overview.mdapps/docs-app/docs/packages/vite-plugin-angular/css-preprocessors.mdpackages/content/devtools/src/index.tspackages/content/package.jsonpackages/content/src/index.spec.tspackages/content/src/index.tspackages/content/vite.config.lib.ts
💤 Files with no reviewable changes (2)
- apps/analog-app/src/app/pages/newsletter.page.ts
- packages/content/src/index.ts
✅ Files skipped from review due to trivial changes (6)
- apps/docs-app/docs/integrations/storybook/index.md
- packages/content/src/index.spec.ts
- packages/content/devtools/src/index.ts
- apps/docs-app/docs/packages/create-analog/overview.md
- apps/docs-app/docs/packages/vite-plugin-angular/css-preprocessors.md
- apps/analog-app/src/routeTree.gen.ts
|
Maintainer context from What We Fixed And WhyThis app exposed three separate regressions while exercising production build output through 1. The browser bundle pulled in Vite-only content devtools codeWhat broke: The In workspace development, that meant the app client bundle could resolve through source, pull Vite internals into runtime code, and fail during hydration with errors like What we changed:
Why this is the right fix: The main content package should stay browser-safe. Devtools are optional and build-time oriented, so they need an explicit opt-in entrypoint instead of piggybacking on the default one. 2. The newsletter page used the old action pathWhat broke: The newsletter example still used the older server action shape and had an unsupported template binding. That left the example out of step with the current What we changed:
Why this is the right fix: This example app is part of the regression surface for Analog features. It should demonstrate the current action API, not a legacy path that quietly drifts out of coverage. 3. The no-SSR
|
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/vite-plugin-angular/src/lib/component-resolvers.ts (1)
57-101:⚠️ Potential issue | 🟡 MinorLimit inline template extraction to actual
@Component()decorators.
collectComponentUrls()still walks everyPropertynode in the file, so the newinlineTemplatespath will also pick up unrelated objects likeconst preview = { template: '<div />' }. That makesgetInlineTemplates()disagree withgetAngularComponentMetadata()on the same source and returns false positives for callers expecting component-only metadata.💡 Minimal fix
/** Extract inline `template` strings from Angular component source. */ export function getInlineTemplates(code: string): string[] { - return collectComponentUrls(code).inlineTemplates; + return getAngularComponentMetadata(code).flatMap( + ({ inlineTemplates }) => inlineTemplates, + ); }Please add a regression case with a non-
@Component{ template: '...' }object in the same file.Also applies to: 208-210
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/component-resolvers.ts` around lines 57 - 101, collectComponentUrls is collecting any Property named "template" in the file, causing inlineTemplates to include non-component objects; restrict extraction to only Properties that belong to an ObjectExpression passed as the first argument to a `@Component`(...) decorator. In the Visitor.Property callback (inside collectComponentUrls) check the node's parent chain: ensure the Property's parent is an ObjectExpression which is the first argument of a CallExpression whose callee is Identifier 'Component' (i.e., decorator call), before pushing to inlineTemplates (and similarly for templateUrl/styleUrl handling if needed). Add a regression test that includes a non-@Component const like const preview = { template: '<div/>' } and assert getInlineTemplates() does not include that template while getAngularComponentMetadata()/component extraction still finds real `@Component` templates.
♻️ Duplicate comments (2)
package.json (1)
71-71:⚠️ Potential issue | 🟡 MinorKeep Angular runtime package version strategy consistent.
Line 71 uses
~21.2.6while the rest of the Angular runtime set is exact-pinned (21.2.6). That can introduce patch drift for just@angular/platform-serveracross environments. If intentional, please document why this package is exempt.#!/bin/bash set -euo pipefail python - <<'PY' import json pkg = json.load(open('package.json')) deps = pkg.get('dependencies', {}) print("Angular dependency versions:") for k in sorted([k for k in deps if k.startswith("@angular/")]): print(f" {k}: {deps[k]}") PYExpected result: confirm whether mixed exact/tilde pinning is intentional and acceptable for CI/release reproducibility.
As per coding guidelines for
package.json: focus on workspace-wide tooling impact, dependency changes, and CI/release implications.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` at line 71, The dependency entry for "@angular/platform-server" is using tilde pinning ("~21.2.6") while the rest of the Angular runtime deps are exact-pinned ("21.2.6"), causing inconsistent version strategy; update the "@angular/platform-server" entry in package.json to use the exact version "21.2.6" to match the other `@angular/`* runtime packages (or, if tilde pinning is intentional, add a short documented justification in the repo docs and a CI note referencing "@angular/platform-server" to explain the exemption).packages/router/src/lib/provide-file-router-base.ts (1)
122-139:⚠️ Potential issue | 🟠 MajorBehavioral regression: meta tags and JSON-LD disabled in DEV mode
The
import.meta.env.DEVguard at lines 125-127 returns a no-op, completely disablingupdateMetaTagsOnRouteChangeandupdateJsonLdOnRouteChangeduring development. This means developers won't see their JSON-LD or meta tags while working locally—a significant DX regression for anyone iterating on SEO or structured data.Additionally, a past reviewer flagged that deferring via
queueMicrotaskafterAPP_BOOTSTRAP_LISTENERcould cause meta tags/JSON-LD to be absent during the first SSR render if the server resolves HTML before the microtask fires. The retrieved learning confirms there's already a pre-existing JSON-LD SSR failure onalpha, but this timing change could exacerbate or mask related issues.Consider:
- Removing the DEV guard or making it configurable so developers can validate structured data locally.
- Validating SSR prerendering behavior to ensure the subscription is active before the initial
NavigationEnd.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/router/src/lib/provide-file-router-base.ts` around lines 122 - 139, The DEV-only early return in the APP_BOOTSTRAP_LISTENER factory disables updateMetaTagsOnRouteChange and updateJsonLdOnRouteChange during development; remove that import.meta.env.DEV guard (or replace it with a configurable flag) so the factory always wires the router subscriptions, and stop deferring subscription setup via queueMicrotask—create the Router/Meta/DOCUMENT injections and call updateMetaTagsOnRouteChange and updateJsonLdOnRouteChange synchronously (or schedule them after setting up subscriptions but not via queueMicrotask) so the subscription is active before the first NavigationEnd; ensure changes are applied in the factory that returns the bootstrap callback (referencing provide: APP_BOOTSTRAP_LISTENER, useFactory, updateMetaTagsOnRouteChange, updateJsonLdOnRouteChange, queueMicrotask, inject(Router), inject(Meta), inject(DOCUMENT)).
🧹 Nitpick comments (4)
packages/platform/src/lib/content-plugin.ts (1)
194-198: UsenormalizedPathin the debug payload for deterministic diagnostics.You already normalize
pathfor directory checks, but the debug event emits the raw value. EmittingnormalizedPathavoids platform-specific separator drift in debug tooling/e2e logs.Proposed patch
server.ws.send('analog:debug-full-reload', { plugin: 'platform:content-plugin', reason: 'content-file-set-changed', - path, + path: normalizedPath, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/platform/src/lib/content-plugin.ts` around lines 194 - 198, The debug event currently emits the raw path variable which can vary by OS; change the payload to send the already-computed normalizedPath instead. Locate the server.ws.send call (server.ws.send('analog:debug-full-reload', {...})) in the content-plugin code and replace the path property with path: normalizedPath (or rename to normalizedPath in the payload if you prefer) so the debug payload consistently uses the normalizedPath value.packages/platform/src/lib/utils/debug.spec.ts (1)
116-120: Consider adding explicit assertion on the first argument for clarity.The test accesses
callArgs[1]to verifyworkspaceRootforwarding. Adding a brief assertion oncallArgs[0](the debug instances array) would make the test more self-documenting and catch accidental argument reordering.Optional: strengthen the assertion
it('uses provided workspaceRoot for file path', () => { applyDebugOption({ logFile: true }, '/custom/root'); const callArgs = vi.mocked(wrapInstancesForFileLog).mock.calls[0]; + expect(Array.isArray(callArgs[0])).toBe(true); // instances array expect(callArgs[1]).toContain('/custom/root'); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/platform/src/lib/utils/debug.spec.ts` around lines 116 - 120, Add an explicit assertion that the first argument passed to wrapInstancesForFileLog is the debug instances array to prevent silent argument reordering: after calling applyDebugOption({ logFile: true }, '/custom/root') and grabbing callArgs from vi.mocked(wrapInstancesForFileLog).mock.calls[0], assert something about callArgs[0] (e.g., that it's an array and contains the expected debug instance shape or length) before asserting callArgs[1] contains '/custom/root'; reference wrapInstancesForFileLog and applyDebugOption to locate where to add this check in the test.packages/vite-plugin-angular/src/lib/stylesheet-registry.spec.ts (1)
24-33: Consider adding test coverage for@import url(...)form.The
rewriteRelativeCssImportsfunction handles both@import "./..."and@import url("./...")forms, but only the string form is tested here. Given that url()-form rewriting is specifically called out as expected behavior for Tailwind v4 compatibility, a test for that path would strengthen regression coverage.🧪 Suggested additional test case
it('rewrites relative css imports to absolute paths', () => { expect( rewriteRelativeCssImports( '@import "./submenu/submenu.component.css";\n.demo { color: red; }', '/project/src/app/header.component.css', ), ).toBe( '@import "/project/src/app/submenu/submenu.component.css";\n.demo { color: red; }', ); }); + + it('rewrites relative css imports in url() form to absolute paths', () => { + expect( + rewriteRelativeCssImports( + '@import url("./submenu/submenu.component.css");\n.demo { color: red; }', + '/project/src/app/header.component.css', + ), + ).toBe( + '@import url("/project/src/app/submenu/submenu.component.css");\n.demo { color: red; }', + ); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/stylesheet-registry.spec.ts` around lines 24 - 33, Add a new unit test in stylesheet-registry.spec.ts that exercises rewriteRelativeCssImports with the `@import` url(...) form: call rewriteRelativeCssImports with a CSS string containing `@import` url("./submenu/submenu.component.css"); and the same base path used in the existing test (e.g. '/project/src/app/header.component.css'), and assert the returned CSS rewrites the relative url to the absolute path '/project/src/app/submenu/submenu.component.css' while leaving the rest of the CSS intact; ensure the test name clearly states it covers the url(...) import form.apps/tailwind-debug-app/src/app/probes/style-probe.component.spec.ts (1)
23-33: Test name slightly misleadingThe test is named "preserves local state across css-only assertions" but it's actually testing the
increment()method and counter display—not CSS-related behavior. Consider renaming to something like "increments and displays click count" for clarity.That said, the test itself is lightweight and targeted per coding guidelines.
Suggested rename
- it('preserves local state across css-only assertions', () => { + it('increments and displays click count', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/tailwind-debug-app/src/app/probes/style-probe.component.spec.ts` around lines 23 - 33, Rename the test description to accurately reflect what it's asserting: change the it(...) title in style-probe.component.spec.ts from "preserves local state across css-only assertions" to something like "increments and displays click count" so it clearly documents that it calls StyleProbeComponent.increment() and checks the probe-counter text content; update any related test comments to match the new name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/docs-app/docs/guides/migrating.md`:
- Line 184: Update the HMR paragraph that mentions the `hmr` option and
`liveReload` alias to explicitly state that Analog requires Angular v19 or newer
for HMR to work; add a short sentence after the existing line (the one
referencing `hmr` and `liveReload`) noting that Angular v16–v18 have HMR
forcibly disabled at runtime with a console warning and therefore HMR is
unavailable on those versions. Ensure the wording references `hmr` and
`liveReload` so readers see the compatibility note in context.
In `@apps/tailwind-debug-app-e2e/tests/component-css-hmr.spec.ts`:
- Around line 46-48: The test teardown currently calls
replaceProbeColor(BLUE_CLASS) which hard-resets the probe stylesheet to a
constant instead of restoring the original file; change the cleanup in
test.afterAll to restore the exact checked-in contents by using ORIGINAL_CSS (or
the variable that captured the file contents before mutation) — e.g., call
replaceProbeColor(ORIGINAL_CSS) or write the saved ORIGINAL_CSS back so the
probe stylesheet is returned byte-for-byte to its pre-test state (update
references to BLUE_CLASS -> ORIGINAL_CSS in test.afterAll and ensure
ORIGINAL_CSS is captured before any mutation).
In `@apps/tailwind-debug-app/project.json`:
- Around line 43-49: The serve-nitro target currently sets cwd to
"dist/apps/tailwind-debug-app/analog" but doesn't ensure the dist is created;
update the "serve-nitro" target so it builds before serving (e.g., add a
dependsOn: ["build"] to the serve-nitro target or change the "command" to run
the build first like "nx build tailwind-debug-app && node
--unhandled-rejections=throw ./server/index.mjs") so that the
dist/apps/tailwind-debug-app/analog directory exists before using the cwd and
running the "command".
In `@apps/tailwind-debug-app/src/app/app.component.css`:
- Line 1: Stylelint is flagging Tailwind v4's `@reference` at-rule; update your
Stylelint config to allow it by adding the rule "scss/at-rule-no-unknown" with
ignoreAtRules: ["reference"] to your .stylelintrc.json (i.e., set
scss/at-rule-no-unknown to [true, { "ignoreAtRules": ["reference"] }]) or
alternatively suppress the warning locally in the CSS file by adding the
appropriate Stylelint disable comment for the at-rule; target the
scss/at-rule-no-unknown rule and the `@reference` at-rule when making the change.
In `@apps/tailwind-debug-app/src/app/debug/debug-stream.service.ts`:
- Around line 22-27: The appendEntry helper currently just concatenates and
trims, causing duplicate events when snapshots are re-appended; change
appendEntry (and the other merge spots handling snapshots/feeds) to deduplicate
by entry.id before trimming: when merging entries (e.g., in appendEntry and the
other merge locations that add a batch/snapshot of TailwindDebugEventEntry),
build the merged list by preserving insertion order but filtering out any
entries whose id already exists (use a Set of ids), then apply the STREAM_LIMIT
slice; update the same logic in the other merge points referenced in the comment
so reconnected snapshots do not re-add entries with the same id.
In `@apps/tailwind-debug-app/src/app/debug/hmr-diagnostics.ts`:
- Around line 80-96: Wrap persistent writes in writeStoredBreadcrumbs and
writeBootCount with a try/catch to prevent storage errors (e.g.,
QuotaExceededError) from bubbling up; catch any thrown error from
storage.setItem, optionally console.warn() the failure with context (include the
key name and error), and silently ignore the error so the in-memory
breadcrumbs/HMR state continues to function. Ensure you only change the
implementations of writeStoredBreadcrumbs and writeBootCount and do not alter
their signatures.
In `@apps/tailwind-debug-app/src/app/probes/debug-stream-panel.component.ts`:
- Around line 58-63: The Clear button currently calls stream.clear() which only
wipes the live buffer; when paused() is true visibleEntries() comes from
frozenEntries(), so you must also clear the frozen view. Update the Clear
handler (the click wired to stream.clear()) to clear both the live stream and
the frozen entries returned by frozenEntries() — e.g., call stream.clear() and
also clear stream.frozenEntries() (or add/ call a
stream.clearFrozen()/resetFrozen() method if that API doesn't exist) so the UI
is empty while paused and after Clear; apply the same change to the duplicate
handler at the other location referenced.
In `@packages/platform/src/lib/router-plugin.ts`:
- Around line 137-172: The current reportRouteDiagnostics(path) only analyzes
the touched .page.ts file, so when route files are added/removed some
parent/ancestor files (e.g., products.page.ts) never get rechecked; update the
logic that invalidates discovery caches for add/unlink events to iterate
discoverRouteFiles().filter(f => f.endsWith('.page.ts')) and call
reportRouteDiagnostics for each remaining page file (or call
analyzeAnalogRouteFile for each) so all dependent pages are rescanned and
routeDiagnosticCache is updated; ensure you still skip non-.page.ts files and
preserve existing error handling in reportRouteDiagnostics and add a watcher
regression test for add/unlink flows.
---
Outside diff comments:
In `@packages/vite-plugin-angular/src/lib/component-resolvers.ts`:
- Around line 57-101: collectComponentUrls is collecting any Property named
"template" in the file, causing inlineTemplates to include non-component
objects; restrict extraction to only Properties that belong to an
ObjectExpression passed as the first argument to a `@Component`(...) decorator. In
the Visitor.Property callback (inside collectComponentUrls) check the node's
parent chain: ensure the Property's parent is an ObjectExpression which is the
first argument of a CallExpression whose callee is Identifier 'Component' (i.e.,
decorator call), before pushing to inlineTemplates (and similarly for
templateUrl/styleUrl handling if needed). Add a regression test that includes a
non-@Component const like const preview = { template: '<div/>' } and assert
getInlineTemplates() does not include that template while
getAngularComponentMetadata()/component extraction still finds real `@Component`
templates.
---
Duplicate comments:
In `@package.json`:
- Line 71: The dependency entry for "@angular/platform-server" is using tilde
pinning ("~21.2.6") while the rest of the Angular runtime deps are exact-pinned
("21.2.6"), causing inconsistent version strategy; update the
"@angular/platform-server" entry in package.json to use the exact version
"21.2.6" to match the other `@angular/`* runtime packages (or, if tilde pinning is
intentional, add a short documented justification in the repo docs and a CI note
referencing "@angular/platform-server" to explain the exemption).
In `@packages/router/src/lib/provide-file-router-base.ts`:
- Around line 122-139: The DEV-only early return in the APP_BOOTSTRAP_LISTENER
factory disables updateMetaTagsOnRouteChange and updateJsonLdOnRouteChange
during development; remove that import.meta.env.DEV guard (or replace it with a
configurable flag) so the factory always wires the router subscriptions, and
stop deferring subscription setup via queueMicrotask—create the
Router/Meta/DOCUMENT injections and call updateMetaTagsOnRouteChange and
updateJsonLdOnRouteChange synchronously (or schedule them after setting up
subscriptions but not via queueMicrotask) so the subscription is active before
the first NavigationEnd; ensure changes are applied in the factory that returns
the bootstrap callback (referencing provide: APP_BOOTSTRAP_LISTENER, useFactory,
updateMetaTagsOnRouteChange, updateJsonLdOnRouteChange, queueMicrotask,
inject(Router), inject(Meta), inject(DOCUMENT)).
---
Nitpick comments:
In `@apps/tailwind-debug-app/src/app/probes/style-probe.component.spec.ts`:
- Around line 23-33: Rename the test description to accurately reflect what it's
asserting: change the it(...) title in style-probe.component.spec.ts from
"preserves local state across css-only assertions" to something like "increments
and displays click count" so it clearly documents that it calls
StyleProbeComponent.increment() and checks the probe-counter text content;
update any related test comments to match the new name.
In `@packages/platform/src/lib/content-plugin.ts`:
- Around line 194-198: The debug event currently emits the raw path variable
which can vary by OS; change the payload to send the already-computed
normalizedPath instead. Locate the server.ws.send call
(server.ws.send('analog:debug-full-reload', {...})) in the content-plugin code
and replace the path property with path: normalizedPath (or rename to
normalizedPath in the payload if you prefer) so the debug payload consistently
uses the normalizedPath value.
In `@packages/platform/src/lib/utils/debug.spec.ts`:
- Around line 116-120: Add an explicit assertion that the first argument passed
to wrapInstancesForFileLog is the debug instances array to prevent silent
argument reordering: after calling applyDebugOption({ logFile: true },
'/custom/root') and grabbing callArgs from
vi.mocked(wrapInstancesForFileLog).mock.calls[0], assert something about
callArgs[0] (e.g., that it's an array and contains the expected debug instance
shape or length) before asserting callArgs[1] contains '/custom/root'; reference
wrapInstancesForFileLog and applyDebugOption to locate where to add this check
in the test.
In `@packages/vite-plugin-angular/src/lib/stylesheet-registry.spec.ts`:
- Around line 24-33: Add a new unit test in stylesheet-registry.spec.ts that
exercises rewriteRelativeCssImports with the `@import` url(...) form: call
rewriteRelativeCssImports with a CSS string containing `@import`
url("./submenu/submenu.component.css"); and the same base path used in the
existing test (e.g. '/project/src/app/header.component.css'), and assert the
returned CSS rewrites the relative url to the absolute path
'/project/src/app/submenu/submenu.component.css' while leaving the rest of the
CSS intact; ensure the test name clearly states it covers the url(...) import
form.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: afd51ea9-9ce6-4bc2-90f9-68b730ddc2dd
⛔ Files ignored due to path filters (8)
.dagger/src/index.tsis excluded by none and included by none.gitignoreis excluded by none and included by noneapps/tailwind-debug-app/public/analog.svgis excluded by!**/*.svgand included byapps/**apps/tailwind-debug-app/public/favicon.icois excluded by!**/*.icoand included byapps/**changes/2026-04-04-reset-investigation/README.mdis excluded by none and included by noneoxlint.config.tsis excluded by none and included by nonepnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by nonetests/vitest-angular/src/snapshot-serializers/__snapshots__/angular-fixture-snapshot.spec.ts.snapis excluded by!**/*.snapand included bytests/**
📒 Files selected for processing (137)
.github/workflows/ci.ymlapps/analog-app-e2e/tests/legacy-server-action.spec.tsapps/analog-app/oxlint.config.tsapps/analog-app/package.jsonapps/analog-app/src/app/app.config.tsapps/analog-app/src/app/pages/client/(client).page.tsapps/analog-app/src/app/pages/legacy-action.page.tsapps/analog-app/src/app/pages/legacy-action.server.tsapps/analog-app/src/app/pages/newsletter.page.tsapps/analog-app/src/app/pages/newsletter.server.tsapps/analog-app/src/routeTree.gen.tsapps/analog-app/vite.config.tsapps/blog-app/src/app/app.config.tsapps/blog-app/src/routeTree.gen.tsapps/docs-app/docs/guides/idiomatic.mdapps/docs-app/docs/guides/migrating.mdapps/docs-app/docs/integrations/angular-material/index.mdapps/docs-app/docs/integrations/storybook/index.mdapps/docs-app/docs/packages/create-analog/overview.mdapps/docs-app/docs/packages/vite-plugin-angular/css-preprocessors.mdapps/docs-app/sidebars.jsapps/opt-catchall-app/src/app/pages/docs.page.tsapps/opt-catchall-app/src/app/pages/docs/[[...slug]].page.tsapps/opt-catchall-app/src/app/pages/docs/index.page.tsapps/tailwind-debug-app-e2e/package.jsonapps/tailwind-debug-app-e2e/playwright.config.tsapps/tailwind-debug-app-e2e/project.jsonapps/tailwind-debug-app-e2e/tests/component-css-hmr.spec.tsapps/tailwind-debug-app/eslint.config.mjsapps/tailwind-debug-app/index.htmlapps/tailwind-debug-app/package.jsonapps/tailwind-debug-app/postcss.config.mjsapps/tailwind-debug-app/project.jsonapps/tailwind-debug-app/public/.gitkeepapps/tailwind-debug-app/src/app/app.component.cssapps/tailwind-debug-app/src/app/app.component.spec.tsapps/tailwind-debug-app/src/app/app.component.tsapps/tailwind-debug-app/src/app/app.config.server.tsapps/tailwind-debug-app/src/app/app.config.tsapps/tailwind-debug-app/src/app/debug/debug-stream.service.tsapps/tailwind-debug-app/src/app/debug/debug-stream.shared.tsapps/tailwind-debug-app/src/app/debug/hmr-diagnostics.spec.tsapps/tailwind-debug-app/src/app/debug/hmr-diagnostics.tsapps/tailwind-debug-app/src/app/pages/(home).page.tsapps/tailwind-debug-app/src/app/pages/analog-welcome.component.tsapps/tailwind-debug-app/src/app/pages/probe.page.tsapps/tailwind-debug-app/src/app/probes/debug-stream-panel.component.tsapps/tailwind-debug-app/src/app/probes/style-probe.component.cssapps/tailwind-debug-app/src/app/probes/style-probe.component.spec.tsapps/tailwind-debug-app/src/app/probes/style-probe.component.tsapps/tailwind-debug-app/src/app/probes/tailwind-debug-shell.component.cssapps/tailwind-debug-app/src/app/probes/tailwind-debug-shell.component.tsapps/tailwind-debug-app/src/main.server.tsapps/tailwind-debug-app/src/main.tsapps/tailwind-debug-app/src/routeTree.gen.tsapps/tailwind-debug-app/src/server/routes/api/v1/hello.tsapps/tailwind-debug-app/src/server/routes/api/ws/debug-stream.tsapps/tailwind-debug-app/src/styles.cssapps/tailwind-debug-app/src/test-setup.tsapps/tailwind-debug-app/src/vite-env.d.tsapps/tailwind-debug-app/tsconfig.app.jsonapps/tailwind-debug-app/tsconfig.editor.jsonapps/tailwind-debug-app/tsconfig.jsonapps/tailwind-debug-app/tsconfig.spec.jsonapps/tailwind-debug-app/vite.config.tsapps/tanstack-query-app/vite.config.tslibs/my-package/package.jsonlibs/my-package/src/lib/my-package/my-package.spec.tslibs/shared/feature/src/pages/blog/posts.[slug].page.tsnx.jsonpackage.jsonpackages/astro-angular/oxlint.config.tspackages/content/devtools/src/index.tspackages/content/mdc/src/lib/mdc-renderer.directive.spec.tspackages/content/oxlint.config.tspackages/content/package.jsonpackages/content/src/index.spec.tspackages/content/src/index.tspackages/content/src/lib/anchor-navigation.directive.spec.tspackages/content/vite.config.lib.tspackages/create-analog/__tests__/cli.spec.tspackages/create-analog/index.jspackages/nx-plugin/src/generators/app/generator.spec.tspackages/nx-plugin/src/generators/app/lib/add-tailwind-config.tspackages/nx-plugin/src/generators/app/lib/add-tailwind-helpers.tspackages/nx-plugin/src/generators/app/versions/nx_18_X/versions.tspackages/nx-plugin/src/generators/app/versions/tailwind-dependencies.tspackages/platform/oxlint.config.tspackages/platform/src/lib/content-plugin.tspackages/platform/src/lib/options.tspackages/platform/src/lib/platform-plugin.tspackages/platform/src/lib/route-idiom-diagnostics.spec.tspackages/platform/src/lib/route-idiom-diagnostics.tspackages/platform/src/lib/router-plugin.spec.tspackages/platform/src/lib/router-plugin.tspackages/platform/src/lib/utils/debug.spec.tspackages/router/oxlint.config.tspackages/router/server/actions/src/page-server-action-compat.spec.tspackages/router/server/src/render.spec.tspackages/router/src/lib/debug/debug.page.tspackages/router/src/lib/form-action.directive.spec.tspackages/router/src/lib/json-ld.spec.tspackages/router/src/lib/json-ld.tspackages/router/src/lib/meta-tags.spec.tspackages/router/src/lib/meta-tags.tspackages/router/src/lib/provide-file-router-base.tspackages/router/src/lib/provide-file-router.spec.tspackages/router/src/lib/route-builder.tspackages/router/src/lib/routes.spec.tspackages/storybook-angular/README.mdpackages/storybook-angular/oxlint.config.tspackages/storybook-angular/src/lib/preset.spec.tspackages/storybook-angular/src/lib/preset.tspackages/storybook-angular/src/types.tspackages/vite-plugin-angular/oxlint.config.tspackages/vite-plugin-angular/src/lib/angular-jit-plugin.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin-live-reload.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.tspackages/vite-plugin-angular/src/lib/angular-vitest-plugin.spec.tspackages/vite-plugin-angular/src/lib/component-resolvers.spec.tspackages/vite-plugin-angular/src/lib/component-resolvers.tspackages/vite-plugin-angular/src/lib/host.spec.tspackages/vite-plugin-angular/src/lib/host.tspackages/vite-plugin-angular/src/lib/plugins/file-replacements.plugin.tspackages/vite-plugin-angular/src/lib/stylesheet-registry.spec.tspackages/vite-plugin-angular/src/lib/stylesheet-registry.tspackages/vite-plugin-angular/src/lib/utils/debug.spec.tspackages/vite-plugin-angular/src/lib/utils/debug.tspackages/vite-plugin-angular/src/lib/utils/devkit.tspackages/vite-plugin-nitro/src/index.tspackages/vite-plugin-nitro/src/lib/vite-plugin-nitro.spec.tspackages/vite-plugin-nitro/src/lib/vite-plugin-nitro.tspnpm-workspace.yamltests/vitest-angular/src/snapshot-serializers/angular-fixture-snapshot.spec.tstools/scripts/debug-nx-project-graph.mtstools/scripts/verify-route-freshness.mts
💤 Files with no reviewable changes (3)
- apps/analog-app/src/app/pages/newsletter.page.ts
- apps/tanstack-query-app/vite.config.ts
- packages/content/src/index.ts
✅ Files skipped from review due to trivial changes (46)
- apps/tailwind-debug-app-e2e/package.json
- apps/analog-app/package.json
- apps/opt-catchall-app/src/app/pages/docs/index.page.ts
- apps/analog-app-e2e/tests/legacy-server-action.spec.ts
- apps/opt-catchall-app/src/app/pages/docs.page.ts
- apps/tailwind-debug-app/index.html
- apps/tailwind-debug-app/src/app/app.component.spec.ts
- apps/tailwind-debug-app/postcss.config.mjs
- libs/my-package/package.json
- libs/my-package/src/lib/my-package/my-package.spec.ts
- packages/vite-plugin-angular/src/lib/angular-vitest-plugin.spec.ts
- libs/shared/feature/src/pages/blog/posts.[slug].page.ts
- packages/nx-plugin/src/generators/app/versions/nx_18_X/versions.ts
- packages/content/mdc/src/lib/mdc-renderer.directive.spec.ts
- apps/tailwind-debug-app/src/vite-env.d.ts
- apps/tailwind-debug-app/src/app/pages/analog-welcome.component.ts
- apps/docs-app/docs/integrations/storybook/index.md
- packages/router/src/lib/form-action.directive.spec.ts
- apps/docs-app/docs/guides/idiomatic.md
- apps/opt-catchall-app/src/app/pages/docs/[[...slug]].page.ts
- apps/tailwind-debug-app/src/app/app.component.ts
- apps/docs-app/docs/packages/vite-plugin-angular/css-preprocessors.md
- packages/router/src/lib/provide-file-router.spec.ts
- packages/router/src/lib/routes.spec.ts
- packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.spec.ts
- apps/tailwind-debug-app/src/app/probes/tailwind-debug-shell.component.css
- apps/tailwind-debug-app/src/styles.css
- apps/tailwind-debug-app/tsconfig.app.json
- apps/tailwind-debug-app/tsconfig.json
- apps/tailwind-debug-app/tsconfig.editor.json
- apps/tailwind-debug-app/src/app/debug/hmr-diagnostics.spec.ts
- apps/tailwind-debug-app-e2e/project.json
- packages/content/src/index.spec.ts
- tests/vitest-angular/src/snapshot-serializers/angular-fixture-snapshot.spec.ts
- packages/content/vite.config.lib.ts
- packages/vite-plugin-angular/src/lib/utils/devkit.ts
- packages/content/devtools/src/index.ts
- tools/scripts/debug-nx-project-graph.mts
- packages/create-analog/index.js
- pnpm-workspace.yaml
- packages/vite-plugin-angular/src/lib/angular-jit-plugin.ts
- packages/storybook-angular/README.md
- nx.json
- packages/router/server/actions/src/page-server-action-compat.spec.ts
- apps/tailwind-debug-app/package.json
- packages/vite-plugin-nitro/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (21)
- apps/analog-app/vite.config.ts
- apps/analog-app/src/app/pages/newsletter.server.ts
- apps/blog-app/src/app/app.config.ts
- apps/docs-app/docs/packages/create-analog/overview.md
- packages/platform/src/lib/options.ts
- packages/storybook-angular/src/lib/preset.spec.ts
- packages/nx-plugin/src/generators/app/lib/add-tailwind-helpers.ts
- packages/platform/src/lib/platform-plugin.ts
- packages/router/src/lib/route-builder.ts
- packages/vite-plugin-angular/src/lib/plugins/file-replacements.plugin.ts
- packages/storybook-angular/src/lib/preset.ts
- packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.ts
- apps/analog-app/src/app/pages/legacy-action.page.ts
- apps/analog-app/src/app/pages/legacy-action.server.ts
- apps/analog-app/src/app/pages/client/(client).page.ts
- packages/nx-plugin/src/generators/app/versions/tailwind-dependencies.ts
- packages/vite-plugin-angular/src/lib/angular-vite-plugin-live-reload.spec.ts
- packages/vite-plugin-angular/src/lib/host.spec.ts
- packages/vite-plugin-angular/src/lib/utils/debug.spec.ts
- packages/nx-plugin/src/generators/app/lib/add-tailwind-config.ts
- packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts (1)
1166-1201: hasComponent detection tests document expected behavior across compilation paths.These tests serve primarily as documentation—they verify string-based
@Componentdetection works on raw TS but fails on compiled output (where decorators becomeɵɵdefineComponentcalls). This is useful for understanding theuseAngularCompilationAPIbehavioral difference but doesn't exercise actual plugin code.Consider adding a comment noting this is behavioral documentation rather than a regression guard, or extracting into a dedicated
*.doc.spec.tsif the pattern grows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts` around lines 1166 - 1201, The tests under "hasComponent detection" are documentation of expected behavior across compilation paths rather than strict regression tests; update the suite to make that explicit by either (A) adding a short clarifying comment and adjusting the it() titles to state "documentation" (e.g., include "documentation-only" or "behavior doc") or (B) move these two specs into a dedicated doc-spec file (e.g., name ending in .doc.spec.ts) to separate behavioral documentation from regression tests; target the describe name "hasComponent detection", the sample variables rawTs and compiledJs, and the reference to useAngularCompilationAPI/ɵɵdefineComponent when making the change.
🤖 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-angular/src/lib/angular-vite-plugin.spec.ts`:
- Around line 1166-1201: The tests under "hasComponent detection" are
documentation of expected behavior across compilation paths rather than strict
regression tests; update the suite to make that explicit by either (A) adding a
short clarifying comment and adjusting the it() titles to state "documentation"
(e.g., include "documentation-only" or "behavior doc") or (B) move these two
specs into a dedicated doc-spec file (e.g., name ending in .doc.spec.ts) to
separate behavioral documentation from regression tests; target the describe
name "hasComponent detection", the sample variables rawTs and compiledJs, and
the reference to useAngularCompilationAPI/ɵɵdefineComponent when making the
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 81c35074-527e-43e6-a131-bd4c171fb776
📒 Files selected for processing (6)
packages/router/src/lib/provide-file-router-base.tspackages/router/src/lib/provide-file-router.spec.tspackages/router/src/lib/route-builder.tspackages/router/src/lib/routes.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.ts
✅ Files skipped from review due to trivial changes (1)
- packages/router/src/lib/routes.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/router/src/lib/provide-file-router-base.ts
- packages/router/src/lib/route-builder.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts (1)
1233-1259: Replace these tautological checks with realuseAngularCompilationAPIcoverage.Both assertions only prove that the hand-written fixtures do or do not contain the substring
@Component; they never execute the plugin/helper that gates this branch. As written, they will keep passing even if the actual compiled-vs-raw detection regresses.As per coding guidelines,
**/*.spec.{ts,tsx}: Keep tests lightweight and targeted to critical functionality testing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts` around lines 1233 - 1259, The two tests currently only check string contents of rawTs and compiledJs; replace the tautological expects with calls to the actual detection helper used by the plugin (e.g., hasComponent or the plugin's detection function) so you assert behavior of the real code path. Specifically, in the "hasComponent detection" suite, call the exported detection function with rawTs and compiledJs (or run the plugin transform hook that uses useAngularCompilationAPI) and assert it returns true for the raw TypeScript fixture and false for the compiled output; keep the existing fixtures rawTs and compiledJs and update the it() blocks to invoke and assert the real helper rather than using .includes('@Component').
🤖 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-angular/src/lib/angular-vite-plugin.spec.ts`:
- Around line 1233-1259: The two tests currently only check string contents of
rawTs and compiledJs; replace the tautological expects with calls to the actual
detection helper used by the plugin (e.g., hasComponent or the plugin's
detection function) so you assert behavior of the real code path. Specifically,
in the "hasComponent detection" suite, call the exported detection function with
rawTs and compiledJs (or run the plugin transform hook that uses
useAngularCompilationAPI) and assert it returns true for the raw TypeScript
fixture and false for the compiled output; keep the existing fixtures rawTs and
compiledJs and update the it() blocks to invoke and assert the real helper
rather than using .includes('@Component').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e6cfcd1a-cc03-4a2c-9fce-33483d14bddf
📒 Files selected for processing (2)
packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/platform/src/lib/router-plugin.ts (1)
239-243: Verify parent layout diagnostics are refreshed after child route add/unlink.On
add/unlink, discovery caches are invalidated (line 236) so subsequentdiscoverPageRouteFiles()calls return fresh results. However, diagnostics are only run for the touched file — existing parent layouts (e.g.,products.page.ts) won't see theirlayout-without-router-outletdiagnostic re-evaluated until they're edited or the server restarts.The past review comment flagged this, and it was marked as addressed. If the current behavior is intentional (defer parent rescan to next edit/restart for performance), consider documenting this limitation in a code comment for future maintainers.
📝 Optional: Document diagnostic rescan scope
if (event === 'unlink') { routeDiagnosticCache.delete(path); } else { + // Only the touched file is re-analyzed for performance; parent + // layouts get rechecked on their next edit or server restart. reportRouteDiagnostics(path); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/platform/src/lib/router-plugin.ts` around lines 239 - 243, The current add/unlink branch only removes the single-file cache and runs reportRouteDiagnostics(path), which leaves parent layouts (e.g., products.page.ts) with stale "layout-without-router-outlet" diagnostics; update the handler so after invalidating routeDiagnosticCache you call discoverPageRouteFiles() to get fresh routes and then run reportRouteDiagnostics for any affected parent/layout files (or at minimum iterate discovered routes and call reportRouteDiagnostics on parent layout files) to re-evaluate layout diagnostics; if this behavior is intentionally deferred for performance, add a clear comment above the add/unlink branch referencing routeDiagnosticCache, discoverPageRouteFiles(), and reportRouteDiagnostics explaining the limitation and why parent rescans are deferred.packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts (2)
925-1003: Duplicate selector/class name tests rely on module-scoped state without isolation.These tests call
transformmultiple times and expect the plugin's "active graph" to accumulate selectors and class names across calls. This works because the plugin maintains module-level state, but:
- Tests in this suite share state with each other (e.g.,
'demo-card'selector registered at line 940 affects line 948's duplicate check).- If test order changes or tests run in parallel, this could cause flaky failures.
- Earlier tests in this file (like
'throws for selectorless non-page components'at line 850) also calltransformand may pollute the active graph for later tests.Consider resetting the active graph state between tests or extracting a fresh plugin instance per test to ensure isolation.
💡 Suggested pattern for test isolation
it('throws for duplicate selectors in the active graph', () => { + // Get a fresh plugin instance to avoid state pollution from other tests const plugin = angular().find( (p) => p.name === '@analogjs/vite-plugin-angular:template-class-binding-guard', ) as Plugin; const transform = typeof plugin.transform === 'function' ? plugin.transform : (plugin.transform as any)?.handler; + // First transform registers the selector transform.call( { warn: vi.fn() } as any, ` `@Component`({ selector: 'demo-card',Alternatively, if the plugin exposes a reset method or the state is accessible, add cleanup in
afterEach:afterEach(() => { // Reset any module-scoped state in the plugin });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts` around lines 925 - 1003, Tests rely on module-scoped "active graph" state inside the template-class-binding-guard plugin (the transform function returned from angular() / plugin name '@analogjs/vite-plugin-angular:template-class-binding-guard'), causing cross-test pollution and flakiness; fix by isolating state between tests: either ensure each test uses a fresh plugin instance by re-importing/creating plugin via angular() inside the test or add an afterEach that resets the module-level state (call any exposed reset method on the guard, or use the test runtime to clear module cache like vi.resetModules() so a new transform/active graph is created for each test). Ensure the chosen approach targets the transform handler returned from angular() so duplicate selector/class-name checks no longer share state across tests.
416-442: Add error path test coverage forrefreshStylesheetRegistryForFile.The function reads the source file from disk without a try-catch wrapper. While
existsSyncprovides an upfront guard, a race condition is theoretically possible in watch mode if the file is deleted between the check and the read. The test suite only covers the happy path; consider adding a test case for file read failures (e.g.,ENOENTor permission errors). This is minor since HMR watches for style files infrequently encounter such errors, but aligns with error handling patterns elsewhere in this file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts` around lines 416 - 442, Add a failing-read test for refreshStylesheetRegistryForFile that simulates the race where existsSync passes but the subsequent read fails: create an AnalogStylesheetRegistry, register a served stylesheet (using registerServedStylesheet with a temp file path and initial normalizedCode), then stub/mock fs.readFileSync (or the underlying file read used by refreshStylesheetRegistryForFile) to throw an ENOENT or permission error when called, invoke refreshStylesheetRegistryForFile with the same path, and assert the registry’s served content (getServedContent for the publicId) remains unchanged (or matches the intended error-path behavior); ensure the stub is restored/cleaned up and any temp files are removed in finally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 145-147: The pnpm v10 allowlist is missing the package with
lifecycle scripts: add "@swc/core" to the pnpm.onlyBuiltDependencies array in
package.json so its postinstall runs and native binaries are initialized; locate
the pnpm.onlyBuiltDependencies list in package.json and include the "@swc/core"
entry alongside the existing entries (the diff shows "@swc-node/register" and
"@swc/helpers" already present).
---
Nitpick comments:
In `@packages/platform/src/lib/router-plugin.ts`:
- Around line 239-243: The current add/unlink branch only removes the
single-file cache and runs reportRouteDiagnostics(path), which leaves parent
layouts (e.g., products.page.ts) with stale "layout-without-router-outlet"
diagnostics; update the handler so after invalidating routeDiagnosticCache you
call discoverPageRouteFiles() to get fresh routes and then run
reportRouteDiagnostics for any affected parent/layout files (or at minimum
iterate discovered routes and call reportRouteDiagnostics on parent layout
files) to re-evaluate layout diagnostics; if this behavior is intentionally
deferred for performance, add a clear comment above the add/unlink branch
referencing routeDiagnosticCache, discoverPageRouteFiles(), and
reportRouteDiagnostics explaining the limitation and why parent rescans are
deferred.
In `@packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.ts`:
- Around line 925-1003: Tests rely on module-scoped "active graph" state inside
the template-class-binding-guard plugin (the transform function returned from
angular() / plugin name
'@analogjs/vite-plugin-angular:template-class-binding-guard'), causing
cross-test pollution and flakiness; fix by isolating state between tests: either
ensure each test uses a fresh plugin instance by re-importing/creating plugin
via angular() inside the test or add an afterEach that resets the module-level
state (call any exposed reset method on the guard, or use the test runtime to
clear module cache like vi.resetModules() so a new transform/active graph is
created for each test). Ensure the chosen approach targets the transform handler
returned from angular() so duplicate selector/class-name checks no longer share
state across tests.
- Around line 416-442: Add a failing-read test for
refreshStylesheetRegistryForFile that simulates the race where existsSync passes
but the subsequent read fails: create an AnalogStylesheetRegistry, register a
served stylesheet (using registerServedStylesheet with a temp file path and
initial normalizedCode), then stub/mock fs.readFileSync (or the underlying file
read used by refreshStylesheetRegistryForFile) to throw an ENOENT or permission
error when called, invoke refreshStylesheetRegistryForFile with the same path,
and assert the registry’s served content (getServedContent for the publicId)
remains unchanged (or matches the intended error-path behavior); ensure the stub
is restored/cleaned up and any temp files are removed in finally.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1eab99b0-0860-43c8-87b5-e9603dbeb914
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (6)
package.jsonpackages/platform/src/lib/router-plugin.spec.tspackages/platform/src/lib/router-plugin.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.tspackages/vite-plugin-angular/src/lib/angular-vite-plugin.tspnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (1)
- pnpm-workspace.yaml
|
@coderabbitai Still-valid findings that I fixed:
Findings that were already resolved and did not need another fix:
Targeted verification run on this branch:
Follow-up commits:
|
|
Tip For best results, initiate chat on the files or code changes.
Confirmed addressed in this round (14 fixes + 2 already-resolved):
Still outstanding:
✏️ Learnings added
🧠 Learnings used |
… full-reload repro
|
@coderabbitai 9e0be35 adjusted the repro to match tailwindlabs/tailwindcss#19903 instead of asserting the opposite behavior. What changed:
Verification:
|
|
Tip For best results, initiate chat on the files or code changes.
Confirmed changes in
The pivot is well-reasoned: the test now serves as a documented repro signal for tailwindlabs/tailwindcss#19903 rather than asserting behavior that doesn't yet hold. The stack-capture addition in the wiretap is a good addition for providing actionable upstream data. The Outstanding tracking unchanged:
✏️ Learnings added
🧠 Learnings used |
…h empty prerender routes

PR Checklist
Related:
Affected scope
vite-plugin-angularplatform,router,vite-plugin-nitro,content,create-analog, docs, repo tooling, examples, test appsRecommended merge strategy for maintainer [optional]
This branch preserves intentional commit boundaries across multiple packages and follow-up review fixes. A rebase merge keeps that history readable without adding an extra merge commit.
What is the new behavior?
This branch is broader than the PR title alone suggests.
The center of gravity is still Angular component stylesheet HMR in
@analogjs/vite-plugin-angular, but the branch also includes the dedicated reproduction app, route/content/runtime fixes needed to validate that work in-repo, the Tailwind full-reload repro alignment, and the follow-up maintainer review fixes that landed on top.Highlights:
@analogjs/vite-plugin-angularcentralizes stylesheet handling for HMR, expands debug logging controls, improves component stylesheet HMR, restores expectedoptimizeDepsprebundling, cleans up stale metadata on file deletion, removes duplicate Vite-ignore injection, fixes class-binding conflict false positives, and limits component metadata extraction to actual@Component(...)decorators.tailwind-debug-appand its e2e coverage now reproduce the current upstream Tailwind full-reload behavior fromtailwindlabs/tailwindcss#19903instead of asserting the opposite behavior, while the supporting debug stream and diagnostics flow were tightened based on review feedback.@analogjs/platformadds dev-time route idiom diagnostics, clearer debug reload reasons, cached page-route discovery for diagnostics, and full rescans when route graph shape changes.@analogjs/routerkeeps explicit app-route precedence while preserving historical last-wins behavior for equal-priority collisions, and route metadata listeners are now active during app init instead of being deferred past first navigation.@analogjs/vite-plugin-nitrono longer requires an SSR entry forssr: falseapps that explicitly opt out of prerender routes, which unblocks production builds for the repro app.@analogjs/contentkeeps the devtools API on the dedicated@analogjs/content/devtoolsentrypoint instead of the main package entrypoint.create-analogscaffolds the Tailwind v4 PostCSS config needed by the debug workflow.Test plan
This PR description reflects targeted validation that was actually run on the branch; it does not claim a fresh full-workspace validation pass after the latest commit stack rewrite.
nx format:checkpnpm buildpnpm testTargeted verification run on this branch:
pnpm vitest run packages/vite-plugin-angular/src/lib/component-resolvers.spec.ts packages/vite-plugin-angular/src/lib/stylesheet-registry.spec.ts packages/platform/src/lib/router-plugin.spec.ts packages/platform/src/lib/utils/debug.spec.tspnpm nx test tailwind-debug-app --runInBandpnpm vitest run --config packages/vite-plugin-angular/vite.config.ts packages/vite-plugin-angular/src/lib/angular-vite-plugin.spec.tspnpm vitest run --config packages/vite-plugin-angular/vite.config.ts packages/vite-plugin-angular/src/lib/angular-vite-plugin-live-reload.spec.tsCI=1 pnpm nx run tailwind-debug-app-e2e:e2epnpm nx run tailwind-debug-app:build:productionpnpm nx test vite-plugin-nitro --runInBandpnpm oxlint -c oxlint.config.ts --quiet apps/tailwind-debug-app/src/app/debug/debug-stream.service.ts apps/tailwind-debug-app/src/app/debug/hmr-diagnostics.ts apps/tailwind-debug-app/src/app/probes/debug-stream-panel.component.ts apps/tailwind-debug-app/src/app/probes/style-probe.component.spec.ts apps/tailwind-debug-app/vite.config.ts apps/tailwind-debug-app-e2e/tests/component-css-hmr.spec.ts packages/platform/src/lib/content-plugin.ts packages/platform/src/lib/router-plugin.ts packages/platform/src/lib/router-plugin.spec.ts packages/platform/src/lib/utils/debug.spec.ts packages/vite-plugin-angular/src/lib/angular-vite-plugin.ts packages/vite-plugin-angular/src/lib/component-resolvers.ts packages/vite-plugin-angular/src/lib/component-resolvers.spec.ts packages/vite-plugin-angular/src/lib/stylesheet-registry.spec.ts packages/vite-plugin-nitro/src/lib/vite-plugin-nitro.spec.tsDoes this PR introduce a breaking change?
Breaking change:
@analogjs/contentdevtools exports are intentionally no longer re-exported from the main@analogjs/contententrypoint. Consumers must import them from@analogjs/content/devtools.Other information
Current branch stack relative to
alpha(33commits):6ce1d364vite-plugin-angulareba6e08evite-plugin-angular2c49f0a2content8cbb8fd4routerf23ce5a2platform8eec739frouter84002ee3create-analogb9b60be7router09482a9eplatformd8742e39vite-plugin-angular87ad7d21test2f2f7171router162043deplatform605d2150build5156354arouter7c9c2e9echore677a4a69vite-plugin-angular>inside quoted class bindings1683e4eevite-plugin-angular[class.foo]inngClassconflict guard6a6efc4arouter7eafecb9routerb55a4366vite-plugin-angularoptimizeDepsprebundling8a93d0cdvite-plugin-angular002de6c9platform61807f34vite-plugin-angularba044b0fchore135c883ebuildfb701b8fdocs9c6a2966fix779c9c8fplatformc37ff35fvite-plugin-angularbcadc13evite-plugin-angular9e0be352vite-plugin-angularc091cf7avite-plugin-nitrossr: falseapps with empty prerender routes