Repository navigation
fix(compiler): dedupe generated i0 namespace imports against all file identifiers - #558
Conversation
… identifiers A user binding named `i0` (const, namespace import, anything in the file's identifier set) collided with the injected `import * as i0 from '@angular/core'`, producing a duplicate declaration. Upstream's AOT/dts ImportManager (`presetImportManagerForceNamespaceImports`) never reuses an existing namespace import: it mints `i0` and renames to `i0_1`, `i0_2`, ... until the name is free in `SourceFile.identifiers` — every identifier in the original file, decorator positions included. The dts transform runs a second ImportManager over the same original identifier set, so `.d.ts` members use the same alias the JS emit picked. - `NamespaceRegistry` now dedupes every generated alias against `file_identifiers` (all identifier nodes in the original program), never reuses an existing `@angular/core` import, and keeps `next_index` consumed on collisions like ImportManager's `nextUniqueIndex++`. - All `i0`/`ɵfac`/`ɵcmp` emit sites — components, directives, pipes, injectables, NgModules, injectors, services, partial declarations, HMR initializer, setClassMetadata/setClassDebugInfo — reference the registry's alias (`core_namespace` threaded through, ~30 signatures). - `DtsDeclaration` gains `namespace_imports` (alias → module specifier, `@angular/core` included) so consumers can emit the imports the members reference; `input_transform_types` prints `@angular/core` transform types through the same alias (`i0_1.Signal` on collision). - Jit-forced classes preserve decorator-position identifiers (member, `declare` member, param decorators) that `__decorate`/`propDecorators` re-emit, alongside the existing ctorParameters preservation. - Vite plugin `utils/dts.ts` rewritten on oxc-parser (zero regexes): class bodies located via AST, member namespace heads rewritten via TSQualifiedName spans, members appended at end of class body like upstream's `[...members, ...newMembers]`, imports emitted after the last import's line (skipping multiline trailing comments), `.d.mts`/ `.d.cts` covered, canonical alias per module identity (relative specifiers resolved against the source module), and structural per-member idempotency so re-runs can't duplicate. Fixes #509
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 62d47efbaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 250e587316
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review on PR #558: - P1: relative specifiers ("./dep") were emitted verbatim into the .d.ts asset, resolving against the OUTPUT directory. Two source files in different dirs canonicalized to i1/i1_1 but both emitted "./dep". injectDtsDeclarations now takes outputFile and rebases relative specifiers to the asset's directory; canonical entries store the emitted specifier and the reuse check compares against it. - P2: the idempotency check counted instance members — an instance `ɵfac: string` would suppress the generated `static ɵfac`. Only static member names now count as previously injected. - P2: parseMembers dereferenced program.body[0] without an undefined guard; empty/comment-only member strings are now handled explicitly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb0563ab99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Follow-up to the previous commit: rebasing to the source tree was wrong — a published package ships dist/ only, and structure-preserving declaration emit (the dominant mode for vite-plugin-dts / rolldown-plugin-dts / tsdown) mirrors the source layout, so "./dep" compiled under src/a/ correctly reaches dist/a/dep.d.ts. Rebasing would have pointed imports at files outside the package. Relative specifiers are still resolved against sourceFile for identity dedup; under fully-bundled declaration output, two same-named relative modules cannot both resolve — documented as a dts-generator limitation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d2850baa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The dts post-processing pass parsed the emitted declaration file with the JS oxc-parser package — a runtime dependency duplicating what the compiler already carries in Rust. injectDtsDeclarations is now a napi export of dts_inject::inject_dts_declarations in oxc_angular_compiler (SourceType::d_ts + oxc_ast_visit), and vite-plugin/utils/dts.ts is a thin typed wrapper. The oxc-parser dependency is removed from napi/angular-compiler (the e2e/compare harness keeps its own copy). Same contract as before: verbatim specifier emission, resolved-identity dedup, static-only idempotency, multiline-comment-aware import insertion.
Codex review on the previous commit (lines are stale — the logic now
lives in crates/oxc_angular_compiler/src/dts_inject.rs):
- P2: end_of_line_after scanned past syntax sharing the last import's
line ("import …; export declare class Foo {") and spliced the new
import inside the class body. import_insert_position now advances only
over trailing comments and the splice opens its own line, so same-line
declarations stay below the new import. Also fixes "import …; // note"
at EOF previously inserting inside the line comment.
- P2: dotted ambient namespaces (declare namespace Outer.Inner) already
recurse in the Rust port (TSNamespaceDeclarationBody nesting); added a
regression test so the finding is covered either way.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a63595aa5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex review: when a caller omits namespaceImports, an unmapped non-ɵ head like i0.Signal<number> was skipped — the member lands with i0 unresolvable, where the pre-canonicalization helper always emitted the i0 import. The registry reserves i0/i0_N for @angular/core, so heads matching that convention are now inferred as core when the map is empty. i1+ aliases stay untouched: without the map their module is unknowable.
The CORE module name and CORE_ALIAS preferred namespace alias now live in r3::identifiers (next to the other Angular runtime identifiers) and are re-exported from crate::r3. Every real-code literal — the registry's mint site, emitter/linker/HMR fallbacks, the host-bindings call, directive initializer-API tables, dts_type matches, and dts_inject's core-alias convention check — references the constants. Test-only literals are left as-is (they assert on the string values).
1 participant
What
Fixes #509 — a user binding named
i0collided with the injectedimport * as i0 from '@angular/core', producing a duplicate declaration.Verified against vendored ngtsc: both the AOT transform
(
transform/src/transform.ts) and the.d.tstransform(
transform/src/declaration.ts, which receivesoriginal.getSourceFile())use
presetImportManagerForceNamespaceImports. So upstream never reuses anexisting namespace import in AOT/dts output — it mints
i0and renames toi0_1,i0_2, … until free of every identifier in the original file(
SourceFile.identifiers). The second ImportManager dedupes over the sameset, so
.d.tsmembers pick the identical alias by construction.Changes
Compiler
NamespaceRegistrydedupes generated aliases againstfile_identifiers(every identifier node in the original program), neverreuses an existing
@angular/corenamespace import, and consumesnext_indexon collisions likenextUniqueIndex++.core_namespacethreaded through every emit site (~30 fns): components,directives, pipes, injectables, NgModules, injectors, services, partial
declarations, HMR initializer,
setClassMetadata/setClassDebugInfo.DtsDeclaration.namespace_imports(alias → specifier,@angular/coreincluded) lets consumers emit the imports members reference.
input_transform_typesprints core types through the same alias(
i0_1.Signalwheni0is taken) — previously hardcodedi0.declaremember, param decorators) that
__decorate/propDecoratorsre-emit.Vite plugin
utils/dts.tsrewritten onoxc-parser— zero regexes. Class bodies viaAST,
TSQualifiedNameheads rewritten by span, members appended at end ofclass body (
[...members, ...newMembers]upstream order), imports emittedafter the last import's line skipping multiline trailing comments,
.d.mts/.d.ctshandled, canonical alias per module identity (relativespecifiers resolved against the source module), structural per-member
idempotency.
Verification
cargo test -p oxc_angular_compiler --all-features— all greencargo run -p oxc_angular_conformance— 1264/1264pnpm test— 392 tests pass;dts.test.ts24 tests(hardcoded
i0inTypePrinter, relative-specifier provenance,cross-alias idempotency, multiline-comment insertion split)
Fixes #509