fix(compiler): ensure that partially compiled queries can handle forward references - #44113
Closed
petebacondarwin wants to merge 2 commits into
Closed
Conversation
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
petebacondarwin
force-pushed
the
linker-content-children-query-forward-ref-issue
branch
3 times, most recently
from
7b5e12c to
949fca5
Compare
November 8, 2021 14:21
JoostK
reviewed
Nov 8, 2021
Comment thread
packages/compiler/src/render3/partial/api.ts
Outdated
JoostK
reviewed
Nov 8, 2021
Comment thread
...est_cases/r3_compiler_compliance/components_and_directives/queries/view_query_forward_ref.js
Outdated
Comment thread
..._cases/r3_compiler_compliance/components_and_directives/queries/content_query_forward_ref.js
Outdated
This interface will be used in other situations so this change renames it to be more general as `MaybeForwardRefExpression`.
petebacondarwin
force-pushed
the
linker-content-children-query-forward-ref-issue
branch
2 times, most recently
from
42c0b3d to
53a6df1
Compare
November 9, 2021 08:15
petebacondarwin
force-pushed
the
linker-content-children-query-forward-ref-issue
branch
from
53a6df1 to
9b9d0de
Compare
November 9, 2021 08:27
petebacondarwin
force-pushed
the
linker-content-children-query-forward-ref-issue
branch
from
9b9d0de to
e03badd
Compare
November 9, 2021 08:29
petebacondarwin
marked this pull request as draft
November 9, 2021 08:53
petebacondarwin
force-pushed
the
linker-content-children-query-forward-ref-issue
branch
from
e03badd to
14b69b2
Compare
November 9, 2021 09:01
JoostK
approved these changes
Nov 10, 2021
Comment thread
packages/compiler/src/injectable_compiler_2.ts
Outdated
…ard references When a partially compiled component or directive is "linked" in JIT mode, the body of its declaration is evaluated by the JavaScript runtime. If a class is referenced in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the file, then the reference must be wrapped in a `forwardRef()` call. Previously, query predicates were not wrapped correctly in partial declarations causing the code to crash at runtime. In AOT mode, this code is never evaluated but instead transformed as part of the build, so this bug did not become apparent until Angular Material started running JIT mode tests on its distributable output. This change fixes this problem by noting when queries are wrapped in `forwardRef()` calls and ensuring that this gets passed through to partial compilation declarations and then suitably stripped during linking. See angular/components#23882 and angular/components#23907
petebacondarwin
force-pushed
the
linker-content-children-query-forward-ref-issue
branch
from
f40e02c to
c0ad276
Compare
November 10, 2021 14:57
alxhub
approved these changes
Nov 10, 2021
Copy link
Copy Markdown
Contributor
Sorry, something went wrong.
Copy link
Copy Markdown
|
I see that you just added the action: merge label, but the following checks are still failing: If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
Sorry, something went wrong.
Copy link
Copy Markdown
Contributor
|
This PR was merged into the repository by commit 393efa5. |
Sorry, something went wrong.
atscott
pushed a commit
that referenced
this pull request
Nov 10, 2021
…ard references (#44113) When a partially compiled component or directive is "linked" in JIT mode, the body of its declaration is evaluated by the JavaScript runtime. If a class is referenced in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the file, then the reference must be wrapped in a `forwardRef()` call. Previously, query predicates were not wrapped correctly in partial declarations causing the code to crash at runtime. In AOT mode, this code is never evaluated but instead transformed as part of the build, so this bug did not become apparent until Angular Material started running JIT mode tests on its distributable output. This change fixes this problem by noting when queries are wrapped in `forwardRef()` calls and ensuring that this gets passed through to partial compilation declarations and then suitably stripped during linking. See angular/components#23882 and angular/components#23907 PR Close #44113
atscott
pushed a commit
that referenced
this pull request
Nov 10, 2021
…ard references (#44113) When a partially compiled component or directive is "linked" in JIT mode, the body of its declaration is evaluated by the JavaScript runtime. If a class is referenced in a query (e.g. `ViewQuery` or `ContentQuery`) but its definition is later in the file, then the reference must be wrapped in a `forwardRef()` call. Previously, query predicates were not wrapped correctly in partial declarations causing the code to crash at runtime. In AOT mode, this code is never evaluated but instead transformed as part of the build, so this bug did not become apparent until Angular Material started running JIT mode tests on its distributable output. This change fixes this problem by noting when queries are wrapped in `forwardRef()` calls and ensuring that this gets passed through to partial compilation declarations and then suitably stripped during linking. See angular/components#23882 and angular/components#23907 PR Close #44113
devversion
added a commit
to devversion/material2
that referenced
this pull request
Nov 11, 2021
…lation output We updated to Angular v13.0.1 which fixes the partial compilation output for `forwardRef` usages in queries. The fixed partial compilation output will ensure that Angular CDK/Material works with JIT linking at runtime, as done in e.g. Jest. Fix upstream: angular/angular#44113. We will add some testing for the JIT linker to the `angular/components` repository with: angular#23882. That should help ensuring JIT linking does not regress. Fixes angular#23907.
devversion
added a commit
to devversion/material2
that referenced
this pull request
Nov 11, 2021
…lation output Updates to Angular v13.0.1 which fixes the partial compilation output for `forwardRef` usages in queries. The fixed partial compilation output will ensure that Angular CDK/Material works with JIT linking at runtime, as done in e.g. Jest. Fix upstream: angular/angular#44113. We will add some testing for the JIT linker to the `angular/components` repository with: angular#23882. That should help ensuring JIT linking does not regress. Fixes angular#23907.
amysorto
pushed a commit
to angular/components
that referenced
this pull request
Nov 11, 2021
…lation output (#23951) Updates to Angular v13.0.1 which fixes the partial compilation output for `forwardRef` usages in queries. The fixed partial compilation output will ensure that Angular CDK/Material works with JIT linking at runtime, as done in e.g. Jest. Fix upstream: angular/angular#44113. We will add some testing for the JIT linker to the `angular/components` repository with: #23882. That should help ensuring JIT linking does not regress. Fixes #23907.
amysorto
pushed a commit
to angular/components
that referenced
this pull request
Nov 11, 2021
…lation output (#23951) Updates to Angular v13.0.1 which fixes the partial compilation output for `forwardRef` usages in queries. The fixed partial compilation output will ensure that Angular CDK/Material works with JIT linking at runtime, as done in e.g. Jest. Fix upstream: angular/angular#44113. We will add some testing for the JIT linker to the `angular/components` repository with: #23882. That should help ensuring JIT linking does not regress. Fixes #23907. (cherry picked from commit 8cb0f12)
petebacondarwin
deleted the
linker-content-children-query-forward-ref-issue
branch
November 12, 2021 14:32
crisbeto
added a commit
to crisbeto/material.angular.io
that referenced
this pull request
Nov 19, 2021
We're currently locked into version 13.0.0 which has a problem with queries using `forwardRef` which affects several places in the docs like the navigation between docs and API examples. These changes update to 13.0.1 which contains angular/angular#44113 in order to resolve the issue.
crisbeto
added a commit
to angular/material.angular.io
that referenced
this pull request
Nov 19, 2021
We're currently locked into version 13.0.0 which has a problem with queries using `forwardRef` which affects several places in the docs like the navigation between docs and API examples. These changes update to 13.0.1 which contains angular/angular#44113 in order to resolve the issue.
This was referenced Dec 1, 2021
Closed
Copy link
Copy Markdown
|
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. |
Sorry, something went wrong.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Reviewers
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.
When a partially compiled component or directive is "linked" in JIT mode, the body
of its declaration is evaluated by the JavaScript runtime. If a class is referenced
in a query (e.g. ViewQuery or ContentQuery) but its definition is later in the
file, then the reference must be wrapped in a forwardRef() call.
Previously, query predicates were not wrapped correctly in partial declarations
causing the code to crash at runtime. In AOT mode, this code is never evaluated
but instead transformed as part of the build, so this bug did not become apparent
until Angular Material started running JIT mode tests on its distributable output.
This change fixes this problem by noting when queries are wrapped in forwardRef()
calls and ensuring that this gets passed through to partial compilation declarations
and then suitably stripped during linking.
See angular/components#23882 and angular/components#23907
Unfortunately, any libraries that were published before this gets merged will potentially have the problem going forward, if they have forwardRefs in their query definitions. The resolution it to simply build and release a new version. With that in mind, it might be worth landing this in 12.2.x LTS as well.