fix(vite-plugin-angular): import NgModule/tuple exports from their defining file in fastCompile#2370
Conversation
…fining file in fastCompile When fastCompile expands an NgModule's `exports` (or a tuple barrel's members) into synthetic imports, local classes have no `sourcePackage`, so the compiler fell back to the module/tuple's own import specifier. This imported the declaration from the module file (e.g. `./x.module`), which only exports the module — not the component — producing `"MyComponent" is not exported by ".../x.module.ts"`. Resolve the synthetic import specifier from the declaration's own `fileName` in the registry instead, deriving a relative path from the consuming file. External `.d.ts` entries keep falling back to the module specifier since their file path is not a usable import target, and same-file declarations emit no synthetic import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
scope:vite-plugin-angular
📝 WalkthroughWalkthroughThis PR refactors synthetic import specifier resolution in the Angular vite plugin compiler. It introduces two utility functions in Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Deploy Preview for analog-blog ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for analog-docs 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/vite-plugin-angular/src/lib/compiler/ngmodule.spec.ts (1)
133-172: ⚡ Quick winAdd a tuple-barrel import-path regression test next to this NgModule case.
This PR updates synthetic specifier resolution in both NgModule and tuple expansion paths, but this new assertion only guards the NgModule branch. A small tuple-focused case asserting import-from-defining-file (not tuple barrel file) would lock in the second changed path too.
As per coding guidelines, “Include tests which validate behavior for any new functionality added”; the PR objective also states both NgModule and tuple-barrel expansion were changed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/vite-plugin-angular/src/lib/compiler/ngmodule.spec.ts` around lines 133 - 172, Add a new spec next to the existing NgModule test that reproduces the tuple-barrel case: create a button component source (ButtonComponent) and a tuple-barrel file (e.g., export const SHARED = [ButtonComponent]) registered via buildRegistry, then compile an app that imports that tuple (use compile and expectCompiles as in the existing test). Assert the generated output includes an import from the component's defining file (expect(result).toContain 'import { ButtonComponent } from "./button.component"') and does NOT import from the tuple-barrel file (expect(result).not.toContain 'import { ButtonComponent } from "./shared.barrel"'), ensuring tuple expansion resolves to the defining file just like the NgModule path.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/vite-plugin-angular/src/lib/compiler/ngmodule.spec.ts`:
- Around line 133-172: Add a new spec next to the existing NgModule test that
reproduces the tuple-barrel case: create a button component source
(ButtonComponent) and a tuple-barrel file (e.g., export const SHARED =
[ButtonComponent]) registered via buildRegistry, then compile an app that
imports that tuple (use compile and expectCompiles as in the existing test).
Assert the generated output includes an import from the component's defining
file (expect(result).toContain 'import { ButtonComponent } from
"./button.component"') and does NOT import from the tuple-barrel file
(expect(result).not.toContain 'import { ButtonComponent } from
"./shared.barrel"'), ensuring tuple expansion resolves to the defining file just
like the NgModule path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cb113946-19b0-470b-9b84-428112373e6c
📒 Files selected for processing (3)
packages/vite-plugin-angular/src/lib/compiler/compile.tspackages/vite-plugin-angular/src/lib/compiler/ngmodule.spec.tspackages/vite-plugin-angular/src/lib/compiler/utils.ts
1 participant
PR Checklist
Fixes a
fastCompilebug where expanding an NgModule'sexports(or a tuple barrel's members) generated a synthetic import that pointed at the module file rather than the file the class is actually defined in, causing build errors like:Affected scope
vite-plugin-angular(fast compiler —compiler/compile.ts,compiler/utils.ts)What is the new behavior?
When a standalone component imports a local NgModule, the fast compiler expands the module's exports into direct declarations and injects synthetic imports for each exported class. For local classes (no
sourcePackage), it previously fell back to the module's import specifier — but the module file only re-exports the module, not the component, so the import failed.The synthetic import specifier is now resolved from the declaration's own
fileNamein the registry, deriving a relative path from the consuming file (./my.componentinstead of./my.module). Details:.d.tsentries continue to fall back to the module specifier (their file path is not a usable relative import target; package-name/sourcePackageresolution is preserved).imports: [SomeImports]where the const is[A, B] as const) is fixed by the same shared helper.New shared helpers live in
compiler/utils.ts:resolveSyntheticImportSpecifierandderiveRelativeImportPath.Test plan
nx format:check— ranprettier --write/--checkon changed files insteadpnpm buildpnpm test—vitest run packages/vite-plugin-angular/src/lib/compiler(1072 passed, 0 failures; conformance baseline unchanged)./button.component, not./shared.module; confirmed it fails without the fix and passes with itDoes this PR introduce a breaking change?
Other information
Reported via a detailed write-up; no linked issue. The report cited line numbers from a built
compile.js; the live source locations are the NgModule (~497) and tuple (~457) export-expansion branches incompiler/compile.ts.🤖 Generated with Claude Code