Repository navigation
fix(compiler): align five small emit shapes with ngtsc (#623) - #657
Merged
Merged
Conversation
- Invalid ctor deps: a param without a usable token makes the whole factory `ɵɵinvalidFactory()` (full) / `deps: "invalid"` (partial), like ngtsc's `unwrapConstructorDependencies` (annotations/common/src/ di.ts). The linker's `ɵɵinvalidFactoryDep` is unchanged. - NgModule schemas: `ɵmod` no longer lists them. The NgModule handler passes `schemas: []` (ng_module/src/handler.ts, "FW-1004" TODO); setClassMetadata still keeps them. - Directive queries: predicates are pooled (`const _c0 = [...]`), content queries first, like `baseDirectiveFields` in render3/view/compiler.ts. Host binding constants continue the index. - Default-imported tokens stay `Foo`, not `i1.default` (typeToValue's LOCAL reference with `defaultImportStatement`). - `@Inject(TOKEN)` tokens stay as written (`getConstructorDependencies` wraps the decorator argument), not `i1.TOKEN` / `i0.DOCUMENT`. Import elision keeps these imports, also without class metadata. A default import used only as a ctor param type is kept when the factory or metadata names it; without metadata, `@Inject` / `@Attribute` replace the type as the token, so it is dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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. |
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f251c3101
ℹ️ 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".
Brooooooklyn
added a commit
that referenced
this pull request
Oct 29, 2026
… the file #657 gave each directive its own query pool, which started empty and was never handed on, so `["x"]` was declared again for every directive and for the components after it. The directive pool now takes the file's pooled literals and passes them on, like a component's, as ngtsc has one pool per file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brooooooklyn
added a commit
that referenced
this pull request
Oct 30, 2026
…e ngtsc (#660) * fix(constant-pool): share long strings and literals across a file like ngtsc Port Angular's `ConstantPool.getConstLiteral` string rule (packages/compiler/src/constant_pool.ts: `getConstLiteral`, `FixupExpression`, `POOL_INCLUSION_LENGTH_THRESHOLD_FOR_STRINGS`, `isLongStringLiteral`). A string of 50+ UTF-16 units stays inline on its first use; its second use makes one shared `const`, and the first use is patched to read it. Component styles now go through it, as in `compileComponentFromMetadata` (render3/view/compiler.ts: `constantPool.getConstLiteral(literal(style))`). ngtsc compiles a file with one pool. Oxc keeps one pool per class, so the file transform hands the pooled literals from one component's pool to the next. Two components with the same long style now share `_c0`, and two `@ViewChild('x')` queries share one `["x"]`. When a later component shares a style an earlier one has inline, the earlier ɵcmp is re-emitted with the constant and the constant is declared before that class. A component that fails to compile hands no literals on, like it leaves the `_c` index. Also stop counting component-specific names (`Cmp_Template`, ...) in the `_c` index handed to the next class, which shifted its constants. Only the shared `_forTrack` and `arrowFn` functions are numbered across the file (ngtsc's `uniqueName` keeps one count per name), so a class named `_A` no longer shifts the next class's `_c` or `Template` names. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(constant-pool): quote strings in literal keys so shared literals don't collide A literal's pool key joined its strings unquoted, so `["a", "b,str:c,str:d"]` and `["a", "b", "c", "d"]` had the same key. With literals now shared across the classes of a file, the second component read the first one's `attrs` constant. Strings are now quoted and escaped in the key. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(constant-pool): share directive query predicates with the rest of the file #657 gave each directive its own query pool, which started empty and was never handed on, so `["x"]` was declared again for every directive and for the components after it. The directive pool now takes the file's pooled literals and passes them on, like a component's, as ngtsc has one pool per file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(constant-pool): hand claimed view function names on to the next class `$A` and `_A` (or `A_` and `A$`) sanitize to the same prefix, so two classes of a file could both declare `_A_Conditional_0_Template` at the top level, a SyntaxError. ngtsc's one pool per file claims names across classes and numbers the second one `_A_Conditional_0_Template1`. The pooled literals handed from class to class (now `SharedPool`) carry the class-specific names each pool claimed, so the next pool numbers its own the same way. `_forTrack` and `arrowFn` keep the file offset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(constant-pool): number and share pool names file-wide like ngtsc ngtsc compiles a file with one ConstantPool. Three gaps remained: - `_forTrack`/`arrowFn` were numbered from the file's `_c` index. Angular's `uniqueName` counts each base name on its own and `freshName` counts `_c`, so the offset is gone and every claimed name is handed on. - Equal shared functions (`getSharedFunctionReference`: track, arrow and `@defer` deps fns) and pure functions (`getSharedConstant`) were declared again by each class. The shared pool now carries them, so a later class reuses the const the earlier class declared. A deps fn reads an imported class through the identifier in the component's own `imports` (a `WrappedNodeExpr` from `LocalIdentifierStrategy`), and `isEquivalent` compares nodes, so such a function is shared within one component only. A wrapped node is now compared by name and span and emitted as written. - A component compiled its template before its host bindings. Angular's `compileComponentFromMetadata` runs `baseDirectiveFields` (queries, then `createHostBindingsFunction`) first, then the selector attrs and the template, so host constants take the lower `_c` indices and are declared first. The HMR update now carries the host constants the template reads. The linker does the same: host bindings first, with one pool, so the template reuses their pure and arrow functions. Ported from @angular/compiler 22.2.2 `ConstantPool` (constant_pool.ts), `WrappedNodeExpr.isEquivalent` (output_ast.ts) and `compileComponentFromMetadata` / `baseDirectiveFields` (render3/view/compiler.ts), and compiler-cli 22.2.3 `LocalIdentifierStrategy` / `Reference.getIdentityIn` (ngtsc/imports). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(defer): lazy-load an import every component of the file defers An import used only inside `@defer` blocks by two components of the same file stayed eager: a class was deferrable only when its `imports` entry was its single value reference, so a second component listing it kept the static import. Port ngtsc's file-wide rule instead. `DeferredSymbolTracker` counts each specifier's references and `markAsDeferrableCandidate` removes the `imports` entries of components that use the class only inside `@defer` (`registerDeferrableCandidate` in `resolveDeferBlocks`); `canDefer` allows a declaration when no specifier has references left. Another component's marks are found the first time a decision needs them, by parsing its template. A class two entries name is listed once (`seenDeps` in `resolveAllDeferredDependencies`). Angular sources: compiler-cli ngtsc/imports/src/deferred_symbol_tracker.ts and ngtsc/annotations/component/src/handler.ts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(emitter): parenthesize a raw arrow factory when it is called A block-body arrow factory the converter keeps as raw source (e.g. one that declares a `const`) was called bare inside the `@Service` / `@Injectable` factory wrapper: `return () => {...}();` and `factory:() =>() => {...}()`, a SyntaxError. An async arrow kept its syntax but called its own body instead of the factory. ngtsc builds `meta.useFactory.callFn([])` over a WrappedNodeExpr, and `ts.factory.createCallExpression` runs the callee through `parenthesizeLeftSideOfAccess`, which wraps anything that is not a left-hand-side expression. Port that rule for `RawSource` callees in the emitter: arrows (and other non-LHS source) get parens; names, member access, function and class expressions stay bare, like ngtsc. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(component): emit the selector attrs field only before Angular v22 ngtsc 22 has no `attrs` on ɵcmp: angular/angular@d99ab0e040 ("fix(compiler): stop generating unused field") removed it from compileComponentFromMetadata in v22.0.0-next.4, and no runtime reads it. We still pooled the first selector's attributes, which also took a `_cN` index and shifted every later constant. Pool and emit it only when angularVersion is below 22.0.0-next.4 (19.x to 21.x still emit it), and pool it after the query predicates, as v21.2's compileComponentFromMetadata does after baseDirectiveFields. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(defer): expect lazy DepsFns to be shared across the file's classes With an import every component of the file defers now loaded lazily, two classes whose blocks load the same thing build the same DepsFn, and the file-wide pool hands the second one the first's name. ngtsc 22.2.3 emits the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(component): pin pre-v22 attrs after the host bindings' constants v21.2's compileComponentFromMetadata pools attrs after baseDirectiveFields, which compiles the host bindings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(pure-functions): escape strings in pure function keys Two bindings like [k, 'a","b'] and [k, 'a', 'b'] built the same key, so the second reused the first's pure function and got a two-entry array. With the pool now shared across classes this also reached from one component to the next. Quote strings with escaping, as expression_to_key already does. ngtsc's GenericKeyFn doesn't escape either and emits the same wrong sharing; this only changes output for such strings. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes five small places where our output differs from ngtsc (
@angular/compiler-cli22.2.3). Part of #623.ɵɵinvalidFactoryDep(n)ɵɵinvalidFactory()(full) /deps: "invalid"(partial). Port ofunwrapConstructorDependencies(annotations/common/src/di.ts)schemasɵmodɵmod, because the handler passesschemas: [](ng_module/src/handler.ts, FW-1004).setClassMetadatastill lists themconst _c0 = [...], content queries first (baseDirectiveFields, render3/view/compiler.ts). Host binding constants keep counting from therei1.defaultFoo(typeToValue's LOCAL ref plusdefaultImportStatement)@Inject(TOKEN)tokeni1.TOKEN/i0.DOCUMENTTOKEN/DOCUMENT, as written (getConstructorDependencies)Import elision follows these changes:
@Injecttoken is kept, with or without class metadata.@Inject/@Attributereplace the type as the token. Nothing usesFoothen, so its import is dropped, as in ngtsc.Example: default import replaced by
@Inject(no class metadata)Before:
After / ngtsc:
ngtsc parity cases checked
All were run through
performCompilation. ngtsc's "no metadata" mode issupportTestBed: false(full mode only):@Inject(TOKEN) f: Fooon an@Injectableand on a@Component, no metadata:Fooimport dropped@Attribute('x') f: Fooon a@Directive, no metadata: dropped@Optional() f: Fooand plainf: Foo, no metadata: kept (ɵɵinject(Foo, 8)/ɵɵinject(Foo))@Inject(TOKEN) f: Foowith metadata: kept (type: FooinctorParameters)@Injecttokens, in full and partial modeChecks
cargo test --all-features: 3608 passed, 0 failedbuild-test, then vitest: 21 files, 397 tests passed.tsc --noEmitis clean. No TS files changed in this round.default_import_overridden_by_inject_or_attribute_is_elided_without_metadatafails whensrc/is stashed and passes with the change.🤖 Generated with Claude Code