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

Wrap imported CSS in an at scope rule #4613

Merged
merged 7 commits into from
Dec 12, 2023
Merged

Conversation

Rheeseyb
Copy link
Contributor

@Rheeseyb Rheeseyb commented Dec 6, 2023

Problem:
User CSS can affect the editor itself. This was fixed by #4604 (and the follow up patch #4611), but that approach is prone to edge cases.

Fix:
As we are primarily targeting Chrome, we can surround user's CSS in @scope (#canvas-container){ ... } at the point of importing in order to scope it to only within the canvas container itself. There are a small number of selectors (e.g. body and :root) that have to be replaced with the :scope selector, but otherwise we can leave the remaining imported CSS untouched.

I've had to update the version of puppeteer used so that we are now running the tests on a version of Chrome that supports this rule (as previously this was breaking the performance tests)

Notes:

Copy link
Contributor

github-actions bot commented Dec 6, 2023

Try me

Copy link

relativeci bot commented Dec 6, 2023

Job #9572: Bundle Size — 63.6MiB (-0.04%).

d3024f1(current) vs df0030c master#9568(baseline)

Warning

Bundle removed 3 duplicate packages, 75 duplicate packages remaining – View duplicate packages

Bundle metrics  Change 6 changes Regression 1 regression Improvement 3 improvements
                 Current
Job #9572
     Baseline
Job #9568
Improvement  Initial JS 46.37MiB(-0.05%) 46.39MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 43.77% 19.32%
No change  Chunks 27 27
No change  Assets 31 31
Change  Modules 4495(-0.11%) 4500
No change  Duplicate Modules 471 471
Regression  Duplicate Code 30.67%(+0.03%) 30.66%
Improvement  Packages 472(-0.63%) 475
Improvement  Duplicate Packages 75(-3.85%) 78
Bundle size by type  Change 1 change Improvement 1 improvement
                 Current
Job #9572
     Baseline
Job #9568
Improvement  JS 63.59MiB (-0.04%) 63.61MiB
Not changed  HTML 11.54KiB 11.54KiB

View job #9572 reportView feature/at-rule-scoped-css branch activity

Copy link
Contributor

github-actions bot commented Dec 6, 2023

Performance test results:
(Chart1)
(Chart2)

Copy link
Contributor

github-actions bot commented Dec 7, 2023

Performance test results:

1 similar comment
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Performance test results:

@Rheeseyb Rheeseyb changed the title (Draft) Wrap imported CSS in an at scope rule Wrap imported CSS in an at scope rule Dec 8, 2023
@Rheeseyb Rheeseyb marked this pull request as ready for review December 8, 2023 13:07
Copy link
Contributor

@liady liady left a comment

Choose a reason for hiding this comment

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

very nice

@Rheeseyb Rheeseyb merged commit be39784 into master Dec 12, 2023
9 checks passed
@Rheeseyb Rheeseyb deleted the feature/at-rule-scoped-css branch December 12, 2023 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants