Repository navigation
fix(compiler): keep spread arguments in call/new expressions and list foreign-decorated members in propDecorators - #556
Merged
Conversation
Issue #511: two divergences from ngtsc emit. - `Argument::SpreadElement` in call/new expressions dropped the `...`, emitting `f(P)` for `f(...P)`. Both converters now emit `OutputExpression::SpreadElement`. - `propDecorators` skipped members with only non-Angular decorators; ngtsc lists them as `prop: []` (metadata.ts:107 gates on `member.decorators.length > 0`, decorators are filtered afterward). Verified against real ngtsc 22 ngc output. Fixes #511
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: 04a8eb1cda
ℹ️ 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".
Review round: `@a.b.Foo()` / `@(a.Foo)()` callees don't survive ngtsc's `_reflectDecorator` (`isDecoratorIdentifier` requires an identifier or `ns.Name` access), so `member.decorators` is null upstream and the member takes the `undecoratedMetadataExtractor` path. Gate on reflected decorators instead of raw decorator nodes so `input()` members decorated that way still get the synthesized `Input` entry.
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
Fixes #511 — two byte-parity divergences from ngtsc in
setClassMetadata/ decorator metadata emit:Spread arguments were flattened.
Argument::SpreadElementin call andnewexpressions converted the inner expression but dropped the..., sof(...P)emitted asf(P)andnew Box(...P)asnew Box(P). Bothconvert_call_expression_with_optionalandconvert_new_expressionnow wrap the argument inOutputExpression::SpreadElement, which the emitter already prints as...expr.propDecoratorsskipped members whose decorators aren't Angular's. ngtsc lists any member withmember.decorators.length > 0(metadata.ts:107);decoratedClassMemberToMetadatafilters the array to Angular decorators, so a member carrying only foreign/local decorators emitsprop: []. We gated the whole entry on!angular_decorators.is_empty()and emitted nothing. Now gated on!decorators.is_empty(). This also fixes decorated signal-initializer members (@Foo() x = input(...)) — upstream's decorated branch wins, so they emitx: [], not the initializer-API entry.Ground truth verified by compiling repros with real ngtsc (
@angular/compiler-cli22.x): emits{ x: [], y: [{type: Input}] }and{ c: [], d: [], b: [{type: core.Output}] }.Changes
src/output/oxc_converter.rs: preserveSpreadElementin call/new args (issue comment references).src/class_metadata/builders.rs:propDecoratorsentry unconditional on the member having decorators.tests/spread_metadata_emit_test.rs(5 tests): spread preserved in full + partial mode for call andnew;x: []for foreign-decorated and decorated-signal members.decorator_metadata_ngtsc_test.rs(c:[],d:[]) andintegration_test.rs(foreignMember:[]+ insta snapshot) to the ngc-verified shape.Verification
cargo test --all-features: all passcargo check,cargo check --all-features,cargo fmt --all -- --check: cleancargo run -p oxc_angular_conformance: 1264/1264