Skip to content

refactor(core): Consolidates shouldTrigger* methods down to one #58833

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.

Sign up for GitHub

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

Jump to bottom
Closed
thePunderWoman wants to merge 2 commits into from
Closed

refactor(core): Consolidates shouldTrigger* methods down to one #58833

thePunderWoman wants to merge 2 commits into from

Conversation

@thePunderWoman
Copy link
Contributor

@thePunderWoman thePunderWoman commented Nov 22, 2024

This removes potential confusion by relying on one shouldTriggerDeferBlock method everywhere.

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

  • Yes
  • No

@thePunderWoman thePunderWoman added action: review The PR is still awaiting reviews from at least one requested reviewer area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release labels Nov 22, 2024
@ngbot ngbot bot added this to the Backlog milestone Nov 22, 2024
@angular-robot angular-robot bot added area: core Issues related to the framework runtime and removed area: core Issues related to the framework runtime labels Nov 22, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 22, 2024
@angular-robot angular-robot bot added area: core Issues related to the framework runtime and removed area: core Issues related to the framework runtime labels Nov 22, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 22, 2024
AndrewKushnir
AndrewKushnir reviewed Nov 23, 2024
View reviewed changes
@angular-robot angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime and removed area: core Issues related to the framework runtime labels Nov 26, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 26, 2024
@thePunderWoman
refactor(compiler): Adds ingest and flags for defer details
eeaaddb 
This adds TDeferDetailsFlags to indicate the presence of hydration triggers, and any future flags we add to defer.
@angular-robot angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime and removed area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime labels Nov 26, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 26, 2024
@angular-robot angular-robot bot removed the area: compiler Issues related to `ngc`, Angular's template compiler label Nov 26, 2024
@angular-robot angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime and removed area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime labels Nov 26, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 26, 2024
AndrewKushnir
AndrewKushnir approved these changes Nov 27, 2024
View reviewed changes
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.

LGTM, just a few minor comments (in docs and tests).

packages/platform-server/test/incremental_hydration_spec.ts Outdated
Copy link
Contributor

@AndrewKushnir AndrewKushnir Nov 27, 2024

Choose a reason for hiding this comment

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

Do we need to wait for 1 second here? We can probably replace this with:

Suggested change
await timeout(1000); // wait for defer blocks to resolve
await allPendingDynamicImports();

Copy link
Contributor Author

@thePunderWoman thePunderWoman Nov 27, 2024

Choose a reason for hiding this comment

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

I've actually gone through and updated every location with this. Extra bonus: tests run in 1/3rd of the time now.

@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 Nov 27, 2024
@thePunderWoman
refactor(core): Consolidates shouldTrigger* methods down to one
2c52299 
This cleans up the triggering code base and consolidates it down to one
function that outlines the logic. This also resolves the `hydrate when`
behavior issue.

fixes: angular#58709
@angular-robot angular-robot bot added area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime and removed area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime labels Nov 27, 2024
@ngbot ngbot bot modified the milestone: Backlog Nov 27, 2024
@thePunderWoman thePunderWoman 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 Nov 27, 2024
@pkozlowski-opensource
Copy link
Member

pkozlowski-opensource commented Nov 27, 2024

This PR was merged into the repository by commit 30891d8.

The changes were merged into the following branches: main, 19.0.x

pkozlowski-opensource pushed a commit that referenced this pull request Nov 27, 2024
@thePunderWoman @pkozlowski-opensource
refactor(compiler): Adds ingest and flags for defer details (#58833)
d6da631 
This adds TDeferDetailsFlags to indicate the presence of hydration triggers, and any future flags we add to defer.

PR Close #58833
pkozlowski-opensource pushed a commit that referenced this pull request Nov 27, 2024
@thePunderWoman @pkozlowski-opensource
refactor(core): Consolidates shouldTrigger* methods down to one (#58833)
d23a5d5 
This cleans up the triggering code base and consolidates it down to one
function that outlines the logic. This also resolves the `hydrate when`
behavior issue.

fixes: #58709

PR Close #58833
pkozlowski-opensource pushed a commit that referenced this pull request Nov 27, 2024
@thePunderWoman @pkozlowski-opensource
refactor(core): Consolidates shouldTrigger* methods down to one (#58833)
30891d8 
This cleans up the triggering code base and consolidates it down to one
function that outlines the logic. This also resolves the `hydrate when`
behavior issue.

fixes: #58709

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

angular-automatic-lock-bot bot commented Dec 28, 2024

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 Dec 28, 2024
@thePunderWoman thePunderWoman deleted the hydrate-when branch March 6, 2025 20:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@AndrewKushnir AndrewKushnir AndrewKushnir approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler area: core Issues related to the framework runtime target: patch This PR is targeted for the next patch release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

3 participants

@thePunderWoman @pkozlowski-opensource @AndrewKushnir