Skip to content

refactor(core): produce a message about @defer behavior when HMR is enabled#60533

Closed
AndrewKushnir wants to merge 1 commit intoangular:mainfrom
AndrewKushnir:defer_and_hmr
Closed

refactor(core): produce a message about @defer behavior when HMR is enabled#60533
AndrewKushnir wants to merge 1 commit intoangular:mainfrom
AndrewKushnir:defer_and_hmr

Conversation

@AndrewKushnir
Copy link
Copy Markdown
Contributor

@AndrewKushnir AndrewKushnir commented Mar 24, 2025

When the HMR is enabled in Angular, all @defer block dependencies are loaded eagerly, instead of waiting for configured trigger conditions. From the DX perspective, it might be seen as an issue when all dependencies are being loaded eagerly. This commit adds a logic to produce a message into the console to provide more info for developers.

Does this PR introduce a breaking change?

  • Yes
  • No

@AndrewKushnir AndrewKushnir 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 core: defer Issues related to @defer blocks. core: hot module replacement (HMR) labels Mar 24, 2025
@ngbot ngbot bot added this to the Backlog milestone Mar 24, 2025
thePunderWoman
thePunderWoman approved these changes Mar 24, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@thePunderWoman thePunderWoman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

reviewed-for: fw-general, public-api, angular-dev

crisbeto
crisbeto approved these changes Mar 24, 2025
View reviewed changes
@AndrewKushnir
refactor(core): produce a message about @defer behavior when HMR is…
c9a139f 
… enabled

When the HMR is enabled in Angular, all `@defer` block dependencies are loaded
eagerly, instead of waiting for configured trigger conditions. From the DX perspective,
it might be seen as an issue when all dependencies are being loaded eagerly. This commit
adds a logic to produce a message into the console to provide more info for developers.
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 24, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor Author

AndrewKushnir commented Mar 24, 2025

@thePunderWoman @crisbeto thanks for the feedback, I've addressed it and pushed an updated version. There is also a minor change to a golden file (a new error code), so this PR would need a public-api review when you get a chance. Thank you.

@thePunderWoman thePunderWoman removed their request for review March 24, 2025 19:15
@pullapprove pullapprove bot requested a review from atscott March 24, 2025 19:15
@pullapprove pullapprove bot requested a review from crisbeto March 24, 2025 19:21
@pullapprove pullapprove bot requested a review from crisbeto March 24, 2025 20:03
@pullapprove pullapprove bot requested a review from crisbeto March 24, 2025 20:04
crisbeto
crisbeto approved these changes Mar 24, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@crisbeto crisbeto left a comment •
edited by thePunderWoman
Loading

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from crisbeto March 24, 2025 20:06
@thePunderWoman thePunderWoman removed the request for review from crisbeto March 24, 2025 20:15
@thePunderWoman thePunderWoman added action: presubmit The PR is in need of a google3 presubmit and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 24, 2025
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit and removed action: presubmit The PR is in need of a google3 presubmit action: merge The PR is ready for merge by the caretaker labels Mar 24, 2025
@AndrewKushnir
Copy link
Copy Markdown
Contributor Author

AndrewKushnir commented Mar 25, 2025 •
edited
Loading

Presubmit.

@AndrewKushnir
Copy link
Copy Markdown
Contributor Author

AndrewKushnir commented Mar 26, 2025 •
edited
Loading

Caretaker note: the presubmit is "green" (after a rerun of some flaky tests), this PR is ready for merge.

Important note: this PR would require the following steps to land in g3:

  • Land this CL.
  • Merge this PR.
  • Create a sync CL.
  • Patch this CL on top of the sync CL.
  • Re-trigger presubmits.
  • Land the sync CL.

Since there is some extra complexity in landing this change, please merge and sync this change separately (as the only change in a sync CL).

@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker and removed action: presubmit The PR is in need of a google3 presubmit labels Mar 26, 2025
@ngbot
Copy link
Copy Markdown

ngbot bot commented Mar 26, 2025

I see that you just added the action: merge label, but the following checks are still failing:
    failure status "google-internal-tests" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken main, please try rebasing to main and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

thePunderWoman pushed a commit that referenced this pull request Mar 28, 2025
@AndrewKushnir @thePunderWoman
refactor(core): produce a message about @defer behavior when HMR is…
882f96f 
… enabled (#60533)

When the HMR is enabled in Angular, all `@defer` block dependencies are loaded
eagerly, instead of waiting for configured trigger conditions. From the DX perspective,
it might be seen as an issue when all dependencies are being loaded eagerly. This commit
adds a logic to produce a message into the console to provide more info for developers.

PR Close #60533
@thePunderWoman
Copy link
Copy Markdown
Contributor

thePunderWoman commented Mar 28, 2025

This PR was merged into the repository by commit 152261c.

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

@angular-automatic-lock-bot
Copy link
Copy Markdown

angular-automatic-lock-bot bot commented Apr 28, 2025

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 28, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@thePunderWoman thePunderWoman thePunderWoman approved these changes

@crisbeto crisbeto crisbeto approved these changes

Assignees

No one assigned

Labels

action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime core: defer Issues related to @defer blocks. core: hot module replacement (HMR) 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

@AndrewKushnir @thePunderWoman @crisbeto