fix(compiler): make sure selectors inside container queries are correctly scoped #48353

dario-piotrowicz · 2022-12-04T21:33:16Z

resolves #48264

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: #48264

What is the new behavior?

Container queries get correctly shimmed by the emulated view encapsulation

Does this PR introduce a breaking change?

Yes
No

Other information

This PR only allows the container queries to be correctly handled and does not provide any scoping for the container names, I am not sure if that is wanted/needed, I've put a comment on it in the issue regarding this

packages/compiler/test/shadow_css/conatiner_queries_spec.ts

JoostK

Thanks for this!

packages/compiler/test/shadow_css/conatiner_queries_spec.ts

JoostK · 2022-12-05T19:37:19Z

packages/compiler/test/shadow_css/conatiner_queries_spec.ts

+    expect(result).toContain('.item[host-a] {');
+  });
+
+  it('should scope normal selectors inside a named container rules', () => {


I'd like to see a test/assertion for the fact that the container name is not scoped, together with a comment similar to what you posted in the issue.

I've added a full match which ignores indentation via a noIndentation utility function, let me know if that works for you 🙂

dario-piotrowicz · 2022-12-05T20:28:07Z

packages/compiler/test/shadow_css/container_queries_spec.ts

+    // Note that for the time being we are not scoping the container name itself,
+    // this is something that may or may not be done in the future depending
+    // on how the css specs evolve. Currently as of Chrome 107 it seems like shadowDom
+    // boundaries don't effect container queries (thus the scoping wouldn't be needed)
+    // And this aspect of container queries seems to be still under active discussion:
+    // https://github.com/w3c/csswg-drafts/issues/5984


@JoostK this got a bit verbose 😓 , I was wondering if I should just mention the fact but not explaining anything and just link to my comment in the issue instead?

something like:

Suggested change

// Note that for the time being we are not scoping the container name itself,

// this is something that may or may not be done in the future depending

// on how the css specs evolve. Currently as of Chrome 107 it seems like shadowDom

// boundaries don't effect container queries (thus the scoping wouldn't be needed)

// And this aspect of container queries seems to be still under active discussion:

// https://github.com/w3c/csswg-drafts/issues/5984

// Note that for the time being we are not scoping the container name itself,

// for more details see: https://github.com/angular/angular/issues/48264#issuecomment-1336524296

what do you prefer? 🙂

I like what you have in the PR now! 👍

JoostK · 2022-12-05T21:46:46Z

packages/compiler/test/shadow_css/container_queries_spec.ts

+    // Note that for the time being we are not scoping the container name itself,
+    // this is something that may or may not be done in the future depending
+    // on how the css specs evolve. Currently as of Chrome 107 it seems like shadowDom
+    // boundaries don't effect container queries (thus the scoping wouldn't be needed)
+    // And this aspect of container queries seems to be still under active discussion:
+    // https://github.com/w3c/csswg-drafts/issues/5984


I like what you have in the PR now! 👍

ngbot · 2022-12-05T21:47:49Z

I see that you just added the action: merge label, but the following checks are still failing:
     status "ci/circleci: setup" is failing
     status "google-internal-tests" is pending
     missing required status "ci/circleci: build"
     missing required status "ci/circleci: lint"
     missing required status "ci/angular: size"

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.

JoostK · 2022-12-05T21:49:58Z

@dario-piotrowicz Could you please rebase the branch to make CI happy?

dario-piotrowicz · 2022-12-05T21:57:23Z

@dario-piotrowicz Could you please rebase the branch to make CI happy?

done 🙂👍

@container

…ctly scoped improve the emulated shadowDom implementation so that it can correctly scope selectors present inside the @container at-rule (recently added to the css specs) resolves angular#48264

AndrewKushnir · 2022-12-06T02:02:20Z

Presubmit + Global Presubmit.

AndrewKushnir · 2022-12-06T17:41:11Z

Caretaker Note: Please ignore the google-internal-tests status. TGP is "green" after rerun.

AndrewKushnir · 2022-12-06T17:59:04Z

This PR was merged into the repository by commit 4c02395.

@container

…ctly scoped (#48353) improve the emulated shadowDom implementation so that it can correctly scope selectors present inside the @container at-rule (recently added to the css specs) resolves #48264 PR Close #48353

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@angular/animations](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fanimations/15.0.2/15.0.4) | | [@angular/common](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcommon/15.0.2/15.0.4) | | [@angular/compiler](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/15.0.2/15.0.4) | | [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/15.0.2/15.0.4) | | [@angular/core](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fcore/15.0.2/15.0.4) | | [@angular/forms](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fforms/15.0.2/15.0.4) | | [@angular/platform-browser](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/15.0.2/15.0.4) | | [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | patch | [`15.0.2` -> `15.0.4`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/15.0.2/15.0.4) | --- ### Release Notes <details> <summary>angular/angular</summary> ### [`v15.0.4`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1504-2022-12-14) [Compare Source](angular/angular@15.0.3...15.0.4) ##### animations | Commit | Type | Description | | -- | -- | -- | | [6c1064c72f](angular/angular@6c1064c) | fix | fix incorrect handling of camel-case css properties ([#48436](angular/angular#48436)) | ##### common | Commit | Type | Description | | -- | -- | -- | | [f30d18a942](angular/angular@f30d18a) | fix | Fix TestBed.overrideProvider type to include multi ([#48424](angular/angular#48424)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [b55d2dab5d](angular/angular@b55d2da) | fix | evaluate const tuple types statically ([#48091](angular/angular#48091)) | #### Special Thanks Alan Agius, Andrew Kushnir, Andrew Scott, Aristeidis Bampakos, Bob Watson, BrowserPerson, Jens, Jessica Janiuk, Joey Perrott, JoostK, Konstantin Kharitonov, Lukas Matta, Piotr Kowalski, Virginia Dooley, Yannick Baron, dario-piotrowicz, lsst25, piyush132000 and why520crazy  ### [`v15.0.3`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#1503-2022-12-07) [Compare Source](angular/angular@15.0.2...15.0.3) ##### common | Commit | Type | Description | | -- | -- | -- | | [50b1c2bf52](angular/angular@50b1c2b) | fix | Don't generate srcsets with very large sources ([#47997](angular/angular#47997)) | | [bf44dc234a](angular/angular@bf44dc2) | fix | Update `Location` to support base href containing `origin` ([#48327](angular/angular#48327)) | ##### compiler | Commit | Type | Description | | -- | -- | -- | | [9a5d84249a](angular/angular@9a5d842) | fix | make sure selectors inside container queries are correctly scoped ([#48353](angular/angular#48353)) | ##### compiler-cli | Commit | Type | Description | | -- | -- | -- | | [167bc0d163](angular/angular@167bc0d) | fix | Produce diagnostic rather than crash when using invalid hostDirective ([#48314](angular/angular#48314)) | ##### core | Commit | Type | Description | | -- | -- | -- | | [e4dcaa513e](angular/angular@e4dcaa5) | fix | unable to inject ChangeDetectorRef inside host directives ([#48355](angular/angular#48355)) | #### Special Thanks Alan Agius, Alex Castle, Andrew Kushnir, Andrew Scott, Bob Watson, Derek Cormier, Joey Perrott, Konstantin Kharitonov, Kristiyan Kostadinov, Paul Gschwendtner, Pawel Kozlowski, dario-piotrowicz and piyush132000  </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Co-authored-by: cabr2-bot <cabr2.help@gmail.com> Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1684 Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org> Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org> Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>

angular-automatic-lock-bot · 2023-01-06T16:06:24Z

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.}

@container

…ctly scoped (angular#48353) improve the emulated shadowDom implementation so that it can correctly scope selectors present inside the @container at-rule (recently added to the css specs) resolves angular#48264 PR Close angular#48353

pullapprove bot requested a review from JoostK December 4, 2022 21:33

dario-piotrowicz commented Dec 4, 2022

View reviewed changes

packages/compiler/test/shadow_css/conatiner_queries_spec.ts Outdated Show resolved Hide resolved

dario-piotrowicz force-pushed the fix-css-container-queries branch 2 times, most recently from 6c511cd to bf16ca8 Compare December 4, 2022 21:57

dario-piotrowicz changed the title ~~fix(compiler): make sure container queries are correctly handled~~ fix(compiler): make sure selectors inside container queries are correctly scoped Dec 4, 2022

JoostK reviewed Dec 5, 2022

View reviewed changes

dario-piotrowicz force-pushed the fix-css-container-queries branch from bf16ca8 to 7d91981 Compare December 5, 2022 20:23

dario-piotrowicz commented Dec 5, 2022

View reviewed changes

dario-piotrowicz force-pushed the fix-css-container-queries branch from 7d91981 to b3aa8a2 Compare December 5, 2022 20:28

JoostK approved these changes Dec 5, 2022

View reviewed changes

JoostK added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release action: presubmit The PR is in need of a google3 presubmit area: compiler Issues related to `ngc`, Angular's template compiler labels Dec 5, 2022

ngbot bot modified the milestone: Backlog Dec 5, 2022

AndrewKushnir added action: global presubmit The PR is in need of a google3 global presubmit and removed action: presubmit The PR is in need of a google3 presubmit labels Dec 5, 2022

AndrewKushnir self-assigned this Dec 5, 2022

dario-piotrowicz force-pushed the fix-css-container-queries branch from b3aa8a2 to dc54ff9 Compare December 5, 2022 21:56

fix(compiler): make sure selectors inside container queries are corre…

b32c101

…ctly scoped improve the emulated shadowDom implementation so that it can correctly scope selectors present inside the @container at-rule (recently added to the css specs) resolves angular#48264

dario-piotrowicz force-pushed the fix-css-container-queries branch from dc54ff9 to b32c101 Compare December 5, 2022 21:59

AndrewKushnir added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Dec 6, 2022

AndrewKushnir removed the action: global presubmit The PR is in need of a google3 global presubmit label Dec 6, 2022

AndrewKushnir closed this in 4c02395 Dec 6, 2022

angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(compiler): make sure selectors inside container queries are correctly scoped #48353

fix(compiler): make sure selectors inside container queries are correctly scoped #48353

dario-piotrowicz commented Dec 4, 2022 •

edited

JoostK left a comment

JoostK Dec 5, 2022

dario-piotrowicz Dec 5, 2022

dario-piotrowicz Dec 5, 2022

JoostK Dec 5, 2022

JoostK Dec 5, 2022

ngbot bot commented Dec 5, 2022

JoostK commented Dec 5, 2022

dario-piotrowicz commented Dec 5, 2022

AndrewKushnir commented Dec 6, 2022 •

edited

AndrewKushnir commented Dec 6, 2022

AndrewKushnir commented Dec 6, 2022

angular-automatic-lock-bot bot commented Jan 6, 2023

fix(compiler): make sure selectors inside container queries are correctly scoped #48353

fix(compiler): make sure selectors inside container queries are correctly scoped #48353

Conversation

dario-piotrowicz commented Dec 4, 2022 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

JoostK left a comment

Choose a reason for hiding this comment

JoostK Dec 5, 2022

Choose a reason for hiding this comment

dario-piotrowicz Dec 5, 2022

Choose a reason for hiding this comment

dario-piotrowicz Dec 5, 2022

Choose a reason for hiding this comment

JoostK Dec 5, 2022

Choose a reason for hiding this comment

JoostK Dec 5, 2022

Choose a reason for hiding this comment

ngbot bot commented Dec 5, 2022

JoostK commented Dec 5, 2022

dario-piotrowicz commented Dec 5, 2022

AndrewKushnir commented Dec 6, 2022 • edited

AndrewKushnir commented Dec 6, 2022

AndrewKushnir commented Dec 6, 2022

angular-automatic-lock-bot bot commented Jan 6, 2023

dario-piotrowicz commented Dec 4, 2022 •

edited

AndrewKushnir commented Dec 6, 2022 •

edited