Add HMR update function #58205
Closed
Add HMR update function #58205
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
7da5275 to
aeae6dd
Compare
AndrewKushnir
approved these changes
Oct 16, 2024
Contributor
AndrewKushnir
left a comment
There was a problem hiding this comment.
LGTM (left a couple minor, non-blocking comments) 👍
Contributor
There was a problem hiding this comment.
nit: it looks like we have similar logic in packages/compiler-cli/src/ngtsc/imports/src/imported_symbols_tracker.ts, we my consider refactoring it (later, in a separate PR) and reuse in multiple places, so we don't duplicate imported symbols extraction logic.
Contributor
There was a problem hiding this comment.
nit: can we declare this function as a class method?
Suggested change
| private addAllTopLevelIdentifiers = (node: ts.Node) => { | |
| private addAllTopLevelIdentifiers(node: ts.Node) { |
Member
Author
There was a problem hiding this comment.
I had to define it as an arrow function, because it uses this and is being passed in directly in the forEachChild call below.
Updates the runtime code to account for the upcoming changes to `ɵɵreplaceMetadata`. I also had to reorganize how the `angularCoreEnv` was set up, because `ɵɵreplaceMetadata` needs access to it without triggering a circular dependency.
…nction returns Fixes that the output AST's `RecursiveVisitor` wasn't visiting all the nodes when an arrow function has an implicit return. The problem was that we were calling the `visitExpression` method directly, instead of `.visitExpression`. This doesn't affect existing code since the `RecursiveVisitor` isn't used anywhere, but it will affect future HMR code.
…itten Currently only the prefix of namespace imports is configurable, but for HMR we need to know ahead of time what the name of `@angular/core` will be. These changes allow us to rewrite it.
Adds some code to the compiler that will generate the HMR update callback function definition.
Adds the ability to generate the function that replaces the component's metadata during HMR. The HMR update module is a function that is loaded dynamically and as such it has some special considerations: * It isn't bundled, because doing so will result in multiple version of core. * Since it isn't bundled, all dependencies have to be passed in as parameters. These changes include some special logic to determine and output those dependencies. * While HMR is enabled, we have to disable the functionality that generates dynamic imports and drop the dependencies inside `@defer` blocks, because we need to retain the ability to refer to them in case they're needed inside the HMR update function. * The function is returned by the `NgCompiler` as a string for the CLI's sake.
aeae6dd to
912a467
Compare
Member
|
This PR was merged into the repository by commit 231e6ff. The changes were merged into the following branches: main |
devversion
pushed a commit
that referenced
this pull request
Oct 16, 2024
…nction returns (#58205) Fixes that the output AST's `RecursiveVisitor` wasn't visiting all the nodes when an arrow function has an implicit return. The problem was that we were calling the `visitExpression` method directly, instead of `.visitExpression`. This doesn't affect existing code since the `RecursiveVisitor` isn't used anywhere, but it will affect future HMR code. PR Close #58205
devversion
pushed a commit
that referenced
this pull request
Oct 16, 2024
Adds some code to the compiler that will generate the HMR update callback function definition. PR Close #58205
devversion
pushed a commit
that referenced
this pull request
Oct 16, 2024
Adds the ability to generate the function that replaces the component's metadata during HMR. The HMR update module is a function that is loaded dynamically and as such it has some special considerations: * It isn't bundled, because doing so will result in multiple version of core. * Since it isn't bundled, all dependencies have to be passed in as parameters. These changes include some special logic to determine and output those dependencies. * While HMR is enabled, we have to disable the functionality that generates dynamic imports and drop the dependencies inside `@defer` blocks, because we need to retain the ability to refer to them in case they're needed inside the HMR update function. * The function is returned by the `NgCompiler` as a string for the CLI's sake. PR Close #58205
|
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.
3 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 changes that are necessary to generate the HMR update function:
refactor(core): account for new replaceMetadata signature
Updates the runtime code to account for the upcoming changes to
ɵɵreplaceMetadata.I also had to reorganize how the
angularCoreEnvwas set up, becauseɵɵreplaceMetadataneeds access to it without triggering a circular dependency.refactor(compiler): output AST visitor not visiting implicit arrow function returns
Fixes that the output AST's
RecursiveVisitorwasn't visiting all the nodes when an arrow function has an implicit return. The problem was that we were calling thevisitExpressionmethod directly, instead of.visitExpression. This doesn't affect existing code since theRecursiveVisitorisn't used anywhere, but it will affect future HMR code.refactor(compiler-cli): allow namespace import identifiers to be rewritten
Currently only the prefix of namespace imports is configurable, but for HMR we need to know ahead of time what the name of
@angular/corewill be. These changes allow us to rewrite it.refactor(compiler): generate HMR update function
Adds some code to the compiler that will generate the HMR update callback function definition.
feat(compiler-cli): generate the HMR replacement module
Adds the ability to generate the function that replaces the component's metadata during HMR. The HMR update module is a function that is loaded dynamically and as such it has some special considerations:
@deferblocks, because we need to retain the ability to refer to them in case they're needed inside the HMR update function.NgCompileras a string for the CLI's sake.