Skip to content

Simplify attribute interpolations#61557

Closed
crisbeto wants to merge 4 commits intoangular:mainfrom
crisbeto:attr-interpolate
Closed

Simplify attribute interpolations#61557
crisbeto wants to merge 4 commits intoangular:mainfrom
crisbeto:attr-interpolate

Conversation

@crisbeto
Copy link
Copy Markdown
Member

@crisbeto crisbeto commented May 21, 2025

Replaces the existing attribute interpolation instructions with a new format. For example previously we would generate attributeInterpolate(name, ...) whereas now we generate attribute(name, interpolate(...)). This allows us to remove all of the specialized interpolation instructions.

This initial PR only deals with attributes, but if there aren't any breakages, we can roll out the approach to all interpolations, saving us ~40 instructions from the set.

@crisbeto crisbeto added action: review The PR is still awaiting reviews from at least one requested reviewer target: minor This PR is targeted for the next minor release labels May 21, 2025
@angular-robot angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime labels May 21, 2025
@ngbot ngbot bot added this to the Backlog milestone May 21, 2025
@crisbeto crisbeto changed the title Attr interpolate Simplify attribute interpolations May 21, 2025

it('should include field name in case of attribute interpolation', () => {
const message = `Previous value for 'attr.id': 'Expressions: a and initial!'. Current value: 'Expressions: a and changed!'`;
const message = `Expression has changed after it was checked. Previous value: 'initial'. Current value: 'changed'`;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This changed, because the error comes from the interpolate instructions which don't know what attribute they're bound to.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, this makes the error message less useful but I think it is the tradeoff we should make


it('should include field name in case of attribute interpolation', () => {
const message = `Previous value for 'attr.id': 'Expressions: a and initial!'. Current value: 'Expressions: a and changed!'`;
const message = `Expression has changed after it was checked. Previous value: 'initial'. Current value: 'changed'`;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, this makes the error message less useful but I think it is the tradeoff we should make

@crisbeto
Copy link
Copy Markdown
Member Author

Passing TGP

crisbeto added 4 commits May 21, 2025 16:36
Adds the new `interpolate*` instructions that can be passed into other instructions and used to replace our existing flavors of interpolations.
Replaces the attribute interpolation instructions with `attribute` plus the new `interpolateX` instruction. This allows to reduce our overall instruction footprint.
The attribute interpolation instructions aren't used anymore so we can remove them.
Updates the `bindingUpdated` function to handle `NO_CHANGE` instead of throwing. This will allow us to reuse instructions across more cases.
@crisbeto crisbeto force-pushed the attr-interpolate branch from b717dd8 to 7ce1fd5 Compare May 21, 2025 14:36
@crisbeto crisbeto added target: rc This PR is targeted for the next release-candidate action: merge The PR is ready for merge by the caretaker and removed target: minor This PR is targeted for the next minor release action: review The PR is still awaiting reviews from at least one requested reviewer labels May 21, 2025
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
Adds the new `interpolate*` instructions that can be passed into other instructions and used to replace our existing flavors of interpolations.

PR Close #61557
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
)

Replaces the attribute interpolation instructions with `attribute` plus the new `interpolateX` instruction. This allows to reduce our overall instruction footprint.

PR Close #61557
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
The attribute interpolation instructions aren't used anymore so we can remove them.

PR Close #61557
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
Updates the `bindingUpdated` function to handle `NO_CHANGE` instead of throwing. This will allow us to reuse instructions across more cases.

PR Close #61557
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
)

Replaces the attribute interpolation instructions with `attribute` plus the new `interpolateX` instruction. This allows to reduce our overall instruction footprint.

PR Close #61557
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
The attribute interpolation instructions aren't used anymore so we can remove them.

PR Close #61557
thePunderWoman pushed a commit that referenced this pull request May 21, 2025
Updates the `bindingUpdated` function to handle `NO_CHANGE` instead of throwing. This will allow us to reuse instructions across more cases.

PR Close #61557
@thePunderWoman
Copy link
Copy Markdown
Contributor

This PR was merged into the repository by commit 1650a85.

The changes were merged into the following branches: main, 20.0.x

@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime target: rc This PR is targeted for the next release-candidate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants