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(serializer): replace LWC scope tokens (BREAKING CHANGE) #185

Merged
merged 5 commits into from
Jun 8, 2023

Conversation

nolanlawson
Copy link
Contributor

@nolanlawson nolanlawson commented Jun 7, 2023

New version of #184

In serialized HTML, all scope tokens are replaced with __lwc_scope_token__. This is a breaking change because it breaks all existing snapshots, and because we now require the LWC peerDependency of 2.48+.

"@lwc/compiler": "^2.48.0",
"@lwc/engine-dom": "^2.48.0",
"@lwc/engine-server": "^2.48.0",
"@lwc/synthetic-shadow": "^2.48.0",
Copy link
Contributor Author

@nolanlawson nolanlawson Jun 7, 2023

Choose a reason for hiding this comment

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

@lwc/compiler v2.48.0 added the cssScopeTokens return value, which we need. We might as well bump every other LWC peer dep as well, since folks shouldn't be mixing-and-matching different versions of LWC packages.

@nolanlawson nolanlawson force-pushed the nolan/scope-token branch 2 times, most recently from 4e3a7c0 to ff3597f Compare June 7, 2023 17:44
Copy link
Member

@ekashida ekashida left a comment

Choose a reason for hiding this comment

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

Looks good--just one question about whether you considered using the existing format of [lwc:scope-token]?

@nolanlawson
Copy link
Contributor Author

I like it! Let's do lwc:scope-token

@nolanlawson
Copy link
Contributor Author

Ah, unfortunately I just realized we can't do lwc:scope-token because the colon messes up the classes in the serialized CSS. This is invalid CSS.

Screenshot 2023-06-07 at 1 23 09 PM

@nolanlawson
Copy link
Contributor Author

On second thought, how about lwc-xxxxxx? Since the new format we're going to use is lwc-<hash>, maybe it's a little less distracting of a name.

@ekashida
Copy link
Member

ekashida commented Jun 8, 2023

I don't have a strong opinion but I'm leaning towards the previous version because it's self-descriptive.

@nolanlawson
Copy link
Contributor Author

Sounds good, let's stick with the first one then!

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.

2 participants