fix(compiler-cli): use correct module import for types behind a forwardRef
#42887
+162
−6
Conversation
…ardRef` The static interpreter assumed that a foreign function expression would have to be imported from the absolute module specifier that was used for the foreign function itself. This assumption does not hold for the `forwardRef` foreign function resolver, as that extracts the resolved expression from the function's argument, which is not behind the absolute module import of the `forwardRef` function. The prior behavior has worked for the typical usage of `forwardRef`, when it is contained within the same source file as where the static evaluation started. In that case, the resulting reference would incorrectly have an absolute module guess of `@angular/core`, but the local identifier emit strategy was capable of emitting the reference without generating an import using the absolute module guess. In the scenario where the static interpreter would first have to follow a reference to a different source that contained the `forwardRef` would the compilation fail. In that case, there is no local identifier available such that the absolute module emitter would try to locate the imported symbol from `@angular/core`. which fails as the symbol is not exported from there. This commit fixes the issue by checking whether a foreign expression occurs in the same source file as the call expression. If it does, then the absolute module specifier that was used to resolve the call expression is ignored. Fixes #42865
packages/compiler-cli/src/ngtsc/partial_evaluator/test/evaluator_spec.ts
Outdated
Show resolved
Hide resolved
|
LGTM - what about a unit test where the FFR and the FFR-expression and the originating expression were all in different files? |
| (_ref, args) => { | ||
| // Extracts the `Foo` from `() => Foo`. | ||
| return (args[0] as ts.ArrowFunction).body as ts.Expression; | ||
| }); |
Comment on lines
658
to
661
petebacondarwin
Jul 19, 2021
Member
Similar to firstArgFfr it would make this test easier to follow if you moved this inline function to a named function.
| const resolved = evaluator.evaluate(expression, (ref) => { | ||
| // Extract the `Foo` from the return type of the `external` function declaration. | ||
| return ((ref.node as ts.FunctionDeclaration).type as ts.TypeReferenceNode).typeName as | ||
| ts.Identifier; | ||
| }); |
Comment on lines
680
to
684
petebacondarwin
Jul 19, 2021
Member
Similarly, it would be easier to follow if this was a named function.
alxhub
added a commit
that referenced
this pull request
Jul 20, 2021
…ardRef` (#42887) The static interpreter assumed that a foreign function expression would have to be imported from the absolute module specifier that was used for the foreign function itself. This assumption does not hold for the `forwardRef` foreign function resolver, as that extracts the resolved expression from the function's argument, which is not behind the absolute module import of the `forwardRef` function. The prior behavior has worked for the typical usage of `forwardRef`, when it is contained within the same source file as where the static evaluation started. In that case, the resulting reference would incorrectly have an absolute module guess of `@angular/core`, but the local identifier emit strategy was capable of emitting the reference without generating an import using the absolute module guess. In the scenario where the static interpreter would first have to follow a reference to a different source that contained the `forwardRef` would the compilation fail. In that case, there is no local identifier available such that the absolute module emitter would try to locate the imported symbol from `@angular/core`. which fails as the symbol is not exported from there. This commit fixes the issue by checking whether a foreign expression occurs in the same source file as the call expression. If it does, then the absolute module specifier that was used to resolve the call expression is ignored. Fixes #42865 PR Close #42887
|
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.
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.
The static interpreter assumed that a foreign function expression would
have to be imported from the absolute module specifier that was used for
the foreign function itself. This assumption does not hold for the
forwardRefforeign function resolver, as that extracts the resolvedexpression from the function's argument, which is not behind the
absolute module import of the
forwardReffunction.The prior behavior has worked for the typical usage of
forwardRef,when it is contained within the same source file as where the static
evaluation started. In that case, the resulting reference would
incorrectly have an absolute module guess of
@angular/core, but thelocal identifier emit strategy was capable of emitting the reference
without generating an import using the absolute module guess.
In the scenario where the static interpreter would first have to follow
a reference to a different source that contained the
forwardRefwouldthe compilation fail. In that case, there is no local identifier
available such that the absolute module emitter would try to locate the
imported symbol from
@angular/core. which fails as the symbol is notexported from there.
This commit fixes the issue by checking whether a foreign expression
occurs in the same source file as the call expression. If it does, then
the absolute module specifier that was used to resolve the call
expression is ignored.
Fixes #42865