Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(compiler): rework defer block analysis #54759

Closed

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Mar 8, 2024

This is a resubmit of #54700.

Currently we have the deferrableDeclToImportDecl, deferBlocks, deferrableTypes and deferBlockDepsEmitMode fields on the R3ComponentMetadata which is incorrect, because the interface is used both for JIT and AOT mode even though the information for those fields is AOT-specific. It will be problematic for partial compilation since the runtime will have a reference to the dependency loading function, but will not be able to provide any of the other information.

These changes make the following refactors:

  1. It changes the defer-related information in R3ComponentMetadata to include only references to dependency functions which can be provided both in JIT and AOT.
  2. Moves the AOT-specific defer analysis into the ComponentResolutionData.
  3. Moves the construction the defer dependency function into the compilation phase of the ComponentDecoratorHandler.
  4. Drops support for defer blocks from the TemplateDefinitionBuilder. This allows us to clean up some TDB-specific code and shouldn't have an effect on users since the TDB isn't used anymore.

@crisbeto crisbeto added the target: major This PR is targeted for the next major release label Mar 8, 2024
@crisbeto
Copy link
Member Author

crisbeto commented Mar 8, 2024

Passing TGP, aside from some unrelated broken targets.

@crisbeto crisbeto added the action: review The PR is still awaiting reviews from at least one requested reviewer label Mar 8, 2024
@crisbeto crisbeto marked this pull request as ready for review March 8, 2024 12:23
Copy link
Contributor

@AndrewKushnir AndrewKushnir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crisbeto looks great 👍

@AndrewKushnir AndrewKushnir added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 9, 2024
@AndrewKushnir
Copy link
Contributor

(adding the "cleanup" label, since there is a merge conflict with the main branch)

@crisbeto crisbeto force-pushed the deferred-partial-refactor-again branch from 57e6b42 to dd1b5a6 Compare March 9, 2024 08:31
crisbeto added a commit to crisbeto/angular that referenced this pull request Mar 9, 2024
Bumps the local dependencies in an attempt to fix the frozen lockfile errors after rebasing some PRs like angular#54759.
crisbeto added a commit to crisbeto/angular that referenced this pull request Mar 9, 2024
Bumps the local dependencies in an attempt to fix the frozen lockfile errors after rebasing some PRs like angular#54759.
crisbeto added a commit to crisbeto/angular that referenced this pull request Mar 9, 2024
Bumps the local dependencies in an attempt to fix the frozen lockfile errors after rebasing some PRs like angular#54759.
crisbeto added a commit to crisbeto/angular that referenced this pull request Mar 9, 2024
Bumps the local dependencies in an attempt to fix the frozen lockfile errors after rebasing some PRs like angular#54759.
crisbeto added a commit to crisbeto/angular that referenced this pull request Mar 9, 2024
Bumps the local dependencies in an attempt to fix the frozen lockfile errors after rebasing some PRs like angular#54759.
crisbeto added a commit to crisbeto/angular that referenced this pull request Mar 9, 2024
Bumps the local dependencies in an attempt to fix the frozen lockfile errors after rebasing some PRs like angular#54759.
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Mar 9, 2024
Currently we have the `deferrableDeclToImportDecl`, `deferBlocks`, `deferrableTypes` and `deferBlockDepsEmitMode` fields on the `R3ComponentMetadata` which is incorrect, because the interface is used both for JIT and AOT mode even though the information for those fields is AOT-specific. It will be problematic for partial compilation since the runtime will have a reference to the dependency loading function, but will not be able to provide any of the other information.

These changes make the following refactors:
1. It changes the defer-related information in `R3ComponentMetadata` to include only references to dependency functions which can be provided both in JIT and AOT.
2. Moves the AOT-specific defer analysis into the `ComponentResolutionData`.
3. Moves the construction the defer dependency function into the compilation phase of the `ComponentDecoratorHandler`.
4. Drops support for defer blocks from the `TemplateDefinitionBuilder`. This allows us to clean up some TDB-specific code and shouldn't have an effect on users since the TDB isn't used anymore.
…ndency

Fixes an issue where we were outputting the reference to non-deferrable dependencies as strings, rather than going through the reference emitter. This caused some issues internally because the reference wasn't maintained in the generated JS.
… package

Moves the logic that creates the defer resolver function into `@angular/compiler` for consistency with the rest of the compilation APIs. Also renames some of the symbols to make it clearer what they're used for.
@crisbeto crisbeto force-pushed the deferred-partial-refactor-again branch from dd1b5a6 to e7ab3eb Compare March 11, 2024 14:52
@atscott
Copy link
Contributor

atscott commented Mar 11, 2024

This PR was merged into the repository by commit 6ea208e.

@atscott atscott closed this in 83932aa Mar 11, 2024
atscott pushed a commit that referenced this pull request Mar 11, 2024
…ndency (#54759)

Fixes an issue where we were outputting the reference to non-deferrable dependencies as strings, rather than going through the reference emitter. This caused some issues internally because the reference wasn't maintained in the generated JS.

PR Close #54759
atscott pushed a commit that referenced this pull request Mar 11, 2024
… package (#54759)

Moves the logic that creates the defer resolver function into `@angular/compiler` for consistency with the rest of the compilation APIs. Also renames some of the symbols to make it clearer what they're used for.

PR Close #54759
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants