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

Fix/shadowrootmode #6933

Merged
merged 5 commits into from
May 23, 2024
Merged

Conversation

spmonahan
Copy link
Contributor

@spmonahan spmonahan commented Apr 5, 2024

Pull Request

πŸ“– Description

Changes the shadowroot attribute on <template>s to shadowrootmode per the standardized declarative shadow DOM when server rendering components.

πŸ‘©β€πŸ’» Reviewer Notes

web.dev notes that pre-standardization, Chrome and Edge used the shadowroot attribute instead. This attribute doesn't work in any current version of the browsers I tested in so I've removed it entirely. This seems a reasonable choice to me since this package is still in beta and the versions of Chrome and Edge that support the non-standard attribute are quite old at this point, however, if it's necessary to support the non-standard attribute I'm happy to do that work but I will need guidance on how to test against older browser versions.

πŸ“‘ Test Plan

I updated relevant tests in the repo and validated the before and after in Edge, Chrome, Safari and Firefox.

This is my test repo.

Before

Edge

edge_shadowroot

Chrome

chrome_shadowroot

Firefox

firefox_shadowroot

Safari

safari_shadowroot

After

Edge

edge_shadowrootmode

Chrome

chrome_shadowrootmode

Firefox

firefox_shadowrootmode

Safari

safari_shadowrootmode

βœ… Checklist

General

  • I have included a change request file using $ yarn change
  • I have added tests for my changes.
  • I have tested my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Copy link
Collaborator

@scomea scomea left a comment

Choose a reason for hiding this comment

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

Seems straightforward

@KingOfTac KingOfTac added the area:ssr Pertains to SSR. label May 23, 2024
@chrisdholt chrisdholt merged commit 4a282ae into microsoft:master May 23, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:ssr Pertains to SSR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants