feat(compiler-cli): lower some exported expressions #30038
Closed
+49
−3
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
The compiler uses metadata to represent what it statically knows about various expressions in a program. Occasionally, expressions in the program for which metadata is extracted may contain sub-expressions which are not representable in metadata. One such construct is an arrow function. The compiler does not always need to understand such expressions completely. For example, for a provider defined with `useValue`, the compiler does not need to understand the value at all, only the outer provider definition. In this case, the compiler employs a technique known as "expression lowering", where it rewrites the provider expression into one that can be represented in metadata. Chiefly, this involves extracting out the dynamic part (the `useValue` expression) into an exported constant. Lowering is applied through a heuristic, which considers the containing statement as well as the field name of the expression. Previously, this heuristic was not completely accurate in the case of route definitions and the `loadChildren` field, which is lowered. If the route definition using `loadChildren` existed inside a decorator invocation, lowering was performed correctly. However, if it existed inside a standalone variable declaration with an export keyword, the heuristic would conclude that lowering was unnecessary. For ordinary providers this is true; however the compiler attempts to fully understand the ROUTES token and thus even if an array of routes is declared in an exported variable, any `loadChildren` expressions within still need to be lowered. This commit enables lowering of already exported variables under a limited set of conditions (where the initializer expression is of a specific form). This should enable the use of `loadChildren` in route definitions.
alxhub
force-pushed
the
ngc/metadata/lowering
branch
from
978026b to
b283720
Compare
alxhub
added
action: merge
alexeagle
reviewed
Apr 22, 2019
| // properly exported. | ||
| const varNode = node as ts.VariableDeclaration; | ||
| return isExported || (varNode.initializer !== undefined && | ||
| (ts.isObjectLiteralExpression(varNode.initializer) || |
There was a problem hiding this comment.
if we really want to reduce risk of this change, we could special-case only the import() callExpression, right? but if you're comfortable with handling more cases, that's great
There was a problem hiding this comment.
It's tricky, because isEligibleForLowering recursively considers the whole statement. Another section of logic determines whether the expression needs lowering (for example, loadChildren: string expressions won't be lowered). This function only sets bounds on which statements are considered in the first place.
alexeagle
approved these changes
Apr 22, 2019
kara
added
the
merge: caretaker note
|
merge-assistance: alex E has global approval and alex R is the code owner for this anyway |
BioPhoton
pushed a commit
to BioPhoton/angular
that referenced
this pull request
May 21, 2019
The compiler uses metadata to represent what it statically knows about various expressions in a program. Occasionally, expressions in the program for which metadata is extracted may contain sub-expressions which are not representable in metadata. One such construct is an arrow function. The compiler does not always need to understand such expressions completely. For example, for a provider defined with `useValue`, the compiler does not need to understand the value at all, only the outer provider definition. In this case, the compiler employs a technique known as "expression lowering", where it rewrites the provider expression into one that can be represented in metadata. Chiefly, this involves extracting out the dynamic part (the `useValue` expression) into an exported constant. Lowering is applied through a heuristic, which considers the containing statement as well as the field name of the expression. Previously, this heuristic was not completely accurate in the case of route definitions and the `loadChildren` field, which is lowered. If the route definition using `loadChildren` existed inside a decorator invocation, lowering was performed correctly. However, if it existed inside a standalone variable declaration with an export keyword, the heuristic would conclude that lowering was unnecessary. For ordinary providers this is true; however the compiler attempts to fully understand the ROUTES token and thus even if an array of routes is declared in an exported variable, any `loadChildren` expressions within still need to be lowered. This commit enables lowering of already exported variables under a limited set of conditions (where the initializer expression is of a specific form). This should enable the use of `loadChildren` in route definitions. PR Close angular#30038
|
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The compiler uses metadata to represent what it statically knows about
various expressions in a program. Occasionally, expressions in the program
for which metadata is extracted may contain sub-expressions which are not
representable in metadata. One such construct is an arrow function.
The compiler does not always need to understand such expressions completely.
For example, for a provider defined with
useValue, the compiler does notneed to understand the value at all, only the outer provider definition. In
this case, the compiler employs a technique known as "expression lowering",
where it rewrites the provider expression into one that can be represented
in metadata. Chiefly, this involves extracting out the dynamic part (the
useValueexpression) into an exported constant.Lowering is applied through a heuristic, which considers the containing
statement as well as the field name of the expression.
Previously, this heuristic was not completely accurate in the case of
route definitions and the
loadChildrenfield, which is lowered. If theroute definition using
loadChildrenexisted inside a decorator invocation,lowering was performed correctly. However, if it existed inside a standalone
variable declaration with an export keyword, the heuristic would conclude
that lowering was unnecessary. For ordinary providers this is true; however
the compiler attempts to fully understand the ROUTES token and thus even if
an array of routes is declared in an exported variable, any
loadChildrenexpressions within still need to be lowered.
This commit enables lowering of already exported variables under a limited
set of conditions (where the initializer expression is of a specific form).
This should enable the use of
loadChildrenin route definitions.PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information