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

feat(style-compiler): enable :root & :host-context() for native-shadow-only CSS #4833

Merged
merged 6 commits into from
Nov 12, 2024

Conversation

cardoso
Copy link
Contributor

@cardoso cardoso commented Nov 11, 2024

Details

Closes #4832

Does this pull request introduce a breaking change?

  • ๐Ÿ˜ฎโ€๐Ÿ’จ No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • ๐Ÿ”ฌ Yes, it does include an observable change.

If disableSyntheticShadowSupport: true is specified, using :root and :host-context() will no longer result in errors.

GUS work item

@cardoso cardoso requested a review from a team as a code owner November 11, 2024 22:30
@cardoso cardoso changed the title feat(style-compiler): enable :root selector for native-shadow-only CSS feat(style-compiler): enable :root & :host-context() for native-shadow-only CSS Nov 11, 2024
Copy link
Collaborator

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

This looks great to me! Thanks for jumping on it so fast. (Impressive! ๐Ÿคฉ)

One small issue I notice here is that, if this is a scoped CSS file (foo.scoped.css) then the selectors are compiled into:

:root.foo {}
:host-context(.foo) {}

These are both valid selectors (TIL) but they don't make much sense for scoped styes. For the :root, you cannot add a class to a shadow root, nor does LWC add a class to the <html> which is where it would need to be at the top level. As for :host-context(), LWC also doesn't add the .foo selector to an ancestor of the host.

I would just have these throw an error if we are in scoped mode. You can test it in the fixtures by adding "scoped": true to the config.json.

Thanks again!!

packages/@lwc/style-compiler/src/postcss-lwc-plugin.ts Outdated Show resolved Hide resolved
@cardoso
Copy link
Contributor Author

cardoso commented Nov 12, 2024

Thanks for the review @nolanlawson ๐Ÿ˜„ I'll work on the scoped case tomorrow.

@cardoso
Copy link
Contributor Author

cardoso commented Nov 12, 2024

@nolanlawson I handled the scoped case in the last commit. I kept it simple, but maybe it'd be better to have different error messages for synthetic & unscoped vs native & scoped?

Copy link
Collaborator

@nolanlawson nolanlawson left a comment

Choose a reason for hiding this comment

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

Thank you! ๐ŸŒˆ

@nolanlawson
Copy link
Collaborator

/nucleus test

@nolanlawson nolanlawson enabled auto-merge (squash) November 12, 2024 21:35
@nolanlawson nolanlawson merged commit c6ca331 into salesforce:master Nov 12, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable :root selector for native-shadow-only CSS
2 participants