Repository navigation
fix(compiler): close the remaining ngtsc parity gaps from #623 - #673
Merged
Merged
Conversation
…ems like ngtsc ngtsc lowercases paths relative to its root dirs when ts.sys.useCaseSensitiveFileNames is false (macOS, Windows), using TypeScript's toFileNameLowerCase. oxc kept the case, and dropped filePath and source locations when a root differed from the file only in case. - TransformOptions.use_case_sensitive_file_names (default true; napi useCaseSensitiveFileNames). When false, the resolved root and file are lowercased like toFileNameLowerCase, for the ɵsetClassDebugInfo filePath and the ɵɵattachSourceLocations path (inline and templateUrl). - Vite plugin: caseSensitiveFileNames option, checked by default the way ts.sys does, on the project's TypeScript install or the plugin file. - e2e compare passes ts.sys.useCaseSensitiveFileNames to oxc. - The HMR id, import paths and the i18n name suffix keep their case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…claration files
A constructor parameter typed by a `const enum` gets `type: undefined` in
the class metadata, like ngtsc's full and partial `typeToValue`. This now
also covers:
- same file: a `const enum` merged with namespaces that hold only types
(a merge with an instantiated namespace is TS2567 and keeps its output)
- namespace imports and member paths: `k.KeyCodes`, `k.NS.Inner`,
`import { NS }` + `NS.Inner`, `b.kk.KeyCodes` through `export * as kk`
- default exports (`export { E as default }`, `export default E`)
- every re-export form, including import-then-export and
`export type { A as B } from`
- `.d.ts` files (with `./x.js` -> `x.d.ts`, `.mjs` -> `.d.mts`,
`.cjs` -> `.d.cts`), and package typings under node_modules
(`exports` "types" condition, "types"/"typings" fields, `main`)
The lookups read each file into a small shape (imports, const enums,
namespaces, exports) and follow a type name the way TypeScript resolves
its symbol. They use their own `export *` walk, which follows only plain
`export *`: the shared walk also entered `export * as ns` sources and
could report a const enum for a regular enum (result depended on map
order). Enum members (`k.KeyCodes.A`) keep their reference. Anything that
can't be followed keeps today's output. Every file read is a dependency
(package files stay out, as before).
The imported const enum paths live in a side table on `StringConsts`, so
`ImportInfo` gets no new field.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A default import named by a ctor param type was always kept, since the
metadata writes `type: Foo` with its local name. When that type is an
imported `const enum`, the metadata now writes `type: undefined`, so the
import was left with no reader (`import KD from './typed'` even pointed
at a module with no JS file). ngtsc drops it.
After the const enum lookup, import elision now removes such a default
import again, when only a top-level class's metadata named it. It stays
when the factory names it (no `@Inject` / `@Attribute`) or a nested
class's metadata does, as before.
Also add a test for a local `export type { E }` of a const enum.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ngtsc builds an NgModule's injector imports from what each entry resolves to. Oxc now does the same where it can read every class an entry names, from this file alone or with cross-file analysis: - a standalone component is left out of `imports` when it brings in no providers (a port of `mayExportProviders`: a `providers` key on an NgModule it reaches counts, a component's own providers don't, and the providers of a `ModuleWithProviders` behind an NgModule aren't read); - an entry kept only in part is written as the classes it keeps, named as this file writes them or through a new namespace import (`i1.X`); - `...LIST` is written as `LIST` when the whole list stays, or when it holds a `ModuleWithProviders`; - `exports` lists each exported NgModule by itself; - `declare const X: readonly [typeof A, ...]` lists of declaration files are read for this. Anything unknown stays as written, and so does an entry whose kept classes are read through `forwardRef` (ngtsc's output can read a class before it exists there), or one with a class its package entry point doesn't export. `ɵɵsetNgModuleScope` keeps the lists as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…omponent imports in ɵinj - A component that is not standalone stays in `ɵinj` imports (ngtsc fails with -992011 here), from this file, another file, a package or a list. - A component whose `imports` can't be read is still a known component: the module that declares `ACmp` keeps its component scope. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…like ngtsc
The partial ɵɵngDeclareNgModule `declarations`, `imports` and `exports`
are now read like ngtsc's `resolveTypeList`: nested arrays, spreads of
array literals and `const` arrays, alias consts, `{ ngModule }` objects
and namespace members (`ns.X`, `ns.X.forRoot()`, `...ns.LIST`) are
flattened, and each ModuleWithProviders is listed as its NgModule.
Duplicates are kept. With cross-file analysis, an imported list or
namespace member is read in its own file and named through `iN.X`;
names this file writes stay as written. Namespace imports are numbered
only after a whole top-level entry resolves, in list order. A top-level
entry oxc can't read keeps its old output.
The `() => [...]` wrap now follows ngtsc's position rule in both modes:
the lists are wrapped when a resolved ref (top-level, nested or found
through another file) is a class of this file declared after the
NgModule. A forwardRef no longer wraps by itself, except when its target
can't be shown to be ready: anything that is not an earlier class of
this file, or an import whose file loads this one back (or any import
without cross-file analysis). Full mode wraps `bootstrap` and the plain
scope lists; lists it emits as written stay unwrapped, because the
runtime only flattens and unwraps a list that is an array.
ɵinj and the full-mode raw scope lists are unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…efined yet
Two cases where the new wrap rule read a class too early, while the
output before did not:
- A forwardRef target imported through a barrel that loads this file
back (`import { FooCmp } from './index'`). Only the class's own file
was checked; now the file the name is imported from is checked too.
- A forwardRef to an earlier class next to a plain entry from a file
that loads this one back. The parser wrapped every list once it saw a
forwardRef, so the wrap now stays then unless every entry is read and
known to be defined (an unread entry keeps it too).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a component's standalone flag or imports, or the module's scope, can't be read, the entry now stays as written instead of being rewritten to the classes that seemed to stay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… process Each transform call has its own analyzer, so the const enum lookups parsed @angular/core's typings (about 670 KB) on every call. Package files are now kept across calls while their modification time and size stay the same (1.1 ms to 0.16 ms per file in a release build). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…th may break - A const list imported from a file that loads this one back keeps the () => wrap a forwardRef asked for: full compilation reads the binding as written, and it may not be initialized yet. - A declaration file's list type in a const list keeps the old output instead of being named as a class. - ɵinj and the partial lists no longer add a relative import into node_modules (a package class reached through a relative export *). ngtsc writes one, but it may not resolve and, under a dev server, loads a second copy of the package: the entry stays as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Member
Author
|
@codex review |
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. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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.
Follow-up to #623 (closed): fixes the gaps left after #670 and #672, checked against ngtsc 22.2.3.
What changes
imports/exportsNgModuleDecoratorHandler.resolve: directives, pipes and standalone components that bring in no providers (mayExportProviders, ported exactly) are left out; a partly kept entry is written as the classes it keeps (i1.X);...LISTbecomesLISTwhen all of it stays. Same file alone, or across files withresolveImportedValues.ɵɵngDeclareNgModuleimports,exportsanddeclarationsresolved like ngtsc (const lists, spreads,ModuleWithProviders,forwardRef, cross-file). The() => [...]wrap follows ngtsc's position rule in partial and full mode.type: undefinedalso through namespaces (k.NS.Inner), every re-export form,export * as,.d.tsfiles and package typings. A default import kept only for such a type is dropped. Package typings are cached for the process (keyed by mtime and size).useCaseSensitiveFileNamesoption (napi) /caseSensitiveFileNames(Vite, auto-detected likets.sys): emitted debug and source-location paths are lowercased on case-insensitive file systems, like ngtsc.Safety rules (where oxc differs from ngtsc on purpose)
importsis a function call, a non-literalstandalone, an unreadable scope).forwardRefwritten as a bare class, a list from a file that loads this one back), the entry or wrap stays.node_modules: ngtsc writes one for a package class reached through a relativeexport *, but it may not resolve and loads a second copy under a dev server. The entry stays.ModuleWithProvidersinexportsis a TypeScript type error; it stays as written, with its providers.convertToTypeArrayreturns a function list unchanged, so wrapping resolved lists would stop flattening); only the wrap changes.iNnumbering can differ from ngtsc; the runtime result is the same.Not in this PR
export * asstar walk bug (cross_file_elision) is not fixed; the const-enum lookups use their own walk.Tests
cargo test -p oxc_angular_compilerwith and without--all-featurespnpm test(439 passed),pnpm check🤖 Generated with Claude Code