Repository navigation
fix(optimizer): keep classes whose definition runs code out of the pure wrapper - #619
Conversation
…re wrapper A private static field, a static block that is not `this.x = …`, an impure superclass or computed key, a static accessor initializer and class decorators all run when the class is defined, so wrapping the class in a pure IIFE let bundlers drop them. Member reads, logical operators and a few globals now count as pure, like the Angular CLI's isPure. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acfb2dea2e
ℹ️ 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".
… keys A member read can run a getter, so a static value such as `registry.value` keeps the class unwrapped. Superclasses and computed keys such as `i0.Base` and `Symbol.iterator` still count as pure: the Angular CLI does not check those at all. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a88d0c4e45
ℹ️ 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".
…class A class followed by `A.ɵfac = f` skipped the class body checks, so an impure superclass, computed key, static field or static block was still wrapped. #604 made `static x = register()` wrap this way; the others were already wrapped before it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 269d4a47a1
ℹ️ 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".
… members out A superclass must now be a declared name, and a computed key a literal, a declared name or `Symbol.<name>`: reading `ns.Base` can run a getter, and turning an object key into a string can run its `toString`. Decorators on class members run when the class is defined, so those classes stay too. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 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". |
Follow-up to #604, fixing the Codex findings on it.
Problem
#604 wraps a class in a
/* @__PURE__ */IIFE when its static members look safe. Some members that run code when the class is defined were not checked. If the class is unused, a bundler drops the IIFE and that code with it.Fix
static #r = register()static { register(A) },static { y = f() },if/varblocksextends registerBase()[register()]() {}(any element)static accessor y = f()@dec export class A {…}@dec export let A = …(does not parse)@register method() {}class A {…}+A.ɵfac = fwith any of the aboveA.ɵfac = f.this.<name> = <wrappable value>.null, a pure class expression, or a call marked/* @__PURE__ */), since readingns.Basecan run a getter. A computed key must be a literal, a declared name orSymbol.<name>, since turning an object into a key can run itstoString.a || band the globalsundefined,NaN,InfinityandSymbolnow count as pure in static values.Checks
class_definition_side_effects_stay.cargo test --workspace: 3410 passed.@angular/*fesm2022 files, linked and then optimized: PR wraps 708 classes, CLI 705. None is wrapped only by the CLI, and every output parses. The 3 extra (Renderer2,DestroyRef,MatButtonHarness) only have arrow or template-string statics.Known limit, shared with the CLI and Babel: a declared name used in
${x},a + bor as a key, or a setter hit bystatic { this.x = … }, could still run user code.Not covered here, and unchanged from main:
export default class, classes inside function bodies, and__decorate/tsickle siblings.🤖 Generated with Claude Code