@let bug fixes #56752
Closed
@let bug fixes #56752
Conversation
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
Fixes that we were only capturing `@let` declarations at the top level of the scope, not any of the nested children.
crisbeto
commented
Jun 28, 2024
Member
Author
There was a problem hiding this comment.
I don't think that this is the most elegant way of solving it, but I think it's likely the most future-proof. Some alternatives that were considered, but didn't work:
- Iterating the instructions in reverse seems to break cases where variables depend on other variables. Otherwise this would've been the simplest.
- We can't hoist the local
@letvariables to the top, because they need anadvancecall.
dylhunn
approved these changes
Jun 28, 2024
thePunderWoman
approved these changes
Jun 28, 2024
Contributor
thePunderWoman
left a comment
There was a problem hiding this comment.
reviewed-for: public-api
atscott
approved these changes
Jun 28, 2024
Contributor
atscott
left a comment
There was a problem hiding this comment.
reviewed-for: public-api
…cal symbols Expands the check around conflicting `@let` declarations to also cover template variables and local references.
alxhub
reviewed
Jun 28, 2024
packages/compiler/src/template/pipeline/src/phases/resolve_names.ts
Outdated
Show resolved
Hide resolved
…ones Currently the logic that maps a name to a variable looks at the variables in their definition order. This means that `@let` declarations from parent views will always come before local ones, because the local ones are declared inline whereas the parent ones are hoisted to the top of the function. These changes resolve the issue by giving precedence to the local variables. Fixes angular#56737.
thePunderWoman
pushed a commit
that referenced
this pull request
Jul 1, 2024
…ones (#56752) Currently the logic that maps a name to a variable looks at the variables in their definition order. This means that `@let` declarations from parent views will always come before local ones, because the local ones are declared inline whereas the parent ones are hoisted to the top of the function. These changes resolve the issue by giving precedence to the local variables. Fixes #56737. PR Close #56752
Contributor
|
This PR was merged into the repository by commit 2a1291e. The changes were merged into the following branches: main |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
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.
Includes the following fixes related to
@let:fix(compiler-cli): type check let declarations nested inside nodes
Fixes that we were only capturing
@letdeclarations at the top level of the scope, not any of the nested children.fix(compiler-cli): flag all conflicts between let declarations and local symbols
Expands the check around conflicting
@letdeclarations to also cover template variables and local references.fix(compiler): give precedence to local let declarations over parent ones
Currently the logic that maps a name to a variable looks at the variables in their definition order. This means that
@letdeclarations from parent views will always come before local ones, because the local ones are declared inline whereas the parent ones are hoisted to the top of the function.These changes resolve the issue by giving precedence to the local variables.
Fixes #56737.