Skip to content

fix(compiler): align five small emit shapes with ngtsc (#623) - #657

Merged
Brooooooklyn merged 1 commit into
mainfrom
fix/ngtsc-emit-alignment
Oct 29, 2026
Merged

Brooooooklyn merged 1 commit into
mainfrom
fix/ngtsc-emit-alignment

Conversation

@Brooooooklyn

Copy link
Copy Markdown
Member

Summary

This fixes five small places where our output differs from ngtsc (@angular/compiler-cli 22.2.3). Part of #623.

# Shape Before After (same as ngtsc)
1 A ctor param with no usable token ɵɵinvalidFactoryDep(n) the whole factory becomes ɵɵinvalidFactory() (full) / deps: "invalid" (partial). Port of unwrapConstructorDependencies (annotations/common/src/di.ts)
2 NgModule schemas listed in ɵmod left out of ɵmod, because the handler passes schemas: [] (ng_module/src/handler.ts, FW-1004). setClassMetadata still lists them
3 Directive query predicates inline arrays pooled as const _c0 = [...], content queries first (baseDirectiveFields, render3/view/compiler.ts). Host binding constants keep counting from there
4 Default-imported token i1.default Foo (typeToValue's LOCAL ref plus defaultImportStatement)
5 @Inject(TOKEN) token i1.TOKEN / i0.DOCUMENT TOKEN / DOCUMENT, as written (getConstructorDependencies)

Import elision follows these changes:

  • The import of an @Inject token is kept, with or without class metadata.
  • A default import used only as a ctor param type is kept when the factory or the metadata uses it.
  • With no metadata, @Inject / @Attribute replace the type as the token. Nothing uses Foo then, so its import is dropped, as in ngtsc.

Example: default import replaced by @Inject (no class metadata)

import Foo from './foo';            // export default interface Foo {}
import { TOKEN } from './tok';
@Injectable() export class S { constructor(@Inject(TOKEN) f: Foo) {} }

Before:

import Foo from './foo';   // kept, but nothing uses it
import { TOKEN } from './tok';
... new (t || S)(i0.ɵɵinject(TOKEN))

After / ngtsc:

import { TOKEN } from './tok';
... new (t || S)(i0.ɵɵinject(TOKEN))

ngtsc parity cases checked

All were run through performCompilation. ngtsc's "no metadata" mode is supportTestBed: false (full mode only):

  • @Inject(TOKEN) f: Foo on an @Injectable and on a @Component, no metadata: Foo import dropped
  • @Attribute('x') f: Foo on a @Directive, no metadata: dropped
  • @Optional() f: Foo and plain f: Foo, no metadata: kept (ɵɵinject(Foo, 8) / ɵɵinject(Foo))
  • @Inject(TOKEN) f: Foo with metadata: kept (type: Foo in ctorParameters)
  • Earlier rounds: invalid deps, schemas, query pooling, default-import tokens and @Inject tokens, in full and partial mode

Checks

  • cargo test --all-features: 3608 passed, 0 failed
  • conformance: 1264/1264, snapshot unchanged
  • clippy: no new warnings on changed lines
  • napi build-test, then vitest: 21 files, 397 tests passed. tsc --noEmit is clean. No TS files changed in this round.
  • The new test default_import_overridden_by_inject_or_attribute_is_elided_without_metadata fails when src/ is stashed and passes with the change.

🤖 Generated with Claude Code

- 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>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T08:06:59.241564Z 9f251c3 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Brooooooklyn

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 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
Brooooooklyn merged commit ea1aa8c into main Oct 29, 2026
10 checks passed
@Brooooooklyn
Brooooooklyn deleted the fix/ngtsc-emit-alignment branch October 29, 2026 02:52
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant