Skip to content

refactor(compiler-cli): add warning for matching viewport @defer t…#65553

Merged
mattrbeck merged 1 commit intoangular:mainfrom
SkyZeroZx:refactor/diagnostic-triggers-defer
Mar 12, 2026
Merged

refactor(compiler-cli): add warning for matching viewport @defer t…#65553
mattrbeck merged 1 commit intoangular:mainfrom
SkyZeroZx:refactor/diagnostic-triggers-defer

Conversation

@SkyZeroZx
Copy link
Contributor

@SkyZeroZx SkyZeroZx commented Nov 22, 2025 •
edited
Loading

…rigger options

Ensures that viewport @defer trigger prefetch warnings occur when the viewport options are identical, including cases with empty or no parameters.

Currently, when using cases like don’t show warning, this PR adds proper support and fixes the missing viewport warning.

With viewport options

    @defer (
     on viewport({   rootMargin: '100px' , threshold: 0.1});
     prefetch on viewport({    threshold: 0.1  rootMargin: '100px'})
   ) {
     <p>Defer Content</p>
   }  @placeholder {
     <p>Placeholder</p>
   }

Using default viewport

   @defer (
    on viewport;
    prefetch on viewport // also viewport({})
  ) {
    <p>Defer Content</p>
  }  @placeholder {
    <p>Placeholder</p>
  }

@pullapprove pullapprove bot requested a review from AndrewKushnir November 22, 2025 21:22
@angular-robot angular-robot bot added the area: compiler Issues related to `ngc`, Angular's template compiler label Nov 22, 2025
@ngbot ngbot bot added this to the Backlog milestone Nov 22, 2025
@JeanMeche JeanMeche added the action: global presubmit The PR is in need of a google3 global presubmit label Nov 22, 2025
@JeanMeche
Copy link
Member

JeanMeche commented Nov 22, 2025

/gemini review

gemini-code-assist[bot]
gemini-code-assist bot reviewed Nov 22, 2025
View reviewed changes
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a warning for @defer blocks when a viewport prefetch trigger has the same options as the main viewport trigger, making it redundant. The implementation correctly handles various cases, including empty and null options. The changes are well-tested. My feedback includes suggestions to improve code conciseness in the new helper function and to reduce boilerplate in the new tests for better maintainability.

@AndrewKushnir AndrewKushnir requested review from JeanMeche and removed request for AndrewKushnir November 24, 2025 01:38
@pullapprove pullapprove bot requested a review from AndrewKushnir November 24, 2025 01:38
@SkyZeroZx SkyZeroZx force-pushed the refactor/diagnostic-triggers-defer branch from 90f422b to 720ce86 Compare November 24, 2025 20:20
@JeanMeche JeanMeche removed the request for review from AndrewKushnir November 24, 2025 21:39
@SkyZeroZx SkyZeroZx force-pushed the refactor/diagnostic-triggers-defer branch from 720ce86 to 4ae9459 Compare December 8, 2025 21:51
@SkyZeroZx
refactor(compiler-cli): add warning for matching viewport @defer trig…
7f2c0e3 
…ger options

Ensures that viewport `@defer` trigger prefetch warnings occur when the viewport options are identical, including cases with empty or no parameters.
@SkyZeroZx SkyZeroZx force-pushed the refactor/diagnostic-triggers-defer branch from 4ae9459 to 7f2c0e3 Compare March 6, 2026 00:18
@SkyZeroZx SkyZeroZx mentioned this pull request Mar 6, 2026
Open
@JeanMeche
Copy link
Member

JeanMeche commented Mar 12, 2026

Passing TGP

@JeanMeche JeanMeche added action: merge The PR is ready for merge by the caretaker target: major This PR is targeted for the next major release labels Mar 12, 2026
@ngbot
Copy link

ngbot bot commented Mar 12, 2026

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.

@mattrbeck mattrbeck merged commit 8edcf41 into angular:main Mar 12, 2026
19 of 21 checks passed
@mattrbeck
Copy link
Member

mattrbeck commented Mar 12, 2026

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JeanMeche JeanMeche JeanMeche approved these changes

+1 more reviewer

@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

action: global presubmit The PR is in need of a google3 global presubmit action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler target: major This PR is targeted for the next major release

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

3 participants

@SkyZeroZx @JeanMeche @mattrbeck