Skip to content

fix: dudupe light scoped issue, hydration coverage#5438

Merged
jhefferman-sfdc merged 7 commits intomasterfrom
jhefferman/dedupe-coverage-scope-token-fix
Aug 1, 2025
Merged

fix: dudupe light scoped issue, hydration coverage#5438
jhefferman-sfdc merged 7 commits intomasterfrom
jhefferman/dedupe-coverage-scope-token-fix

Conversation

@jhefferman-sfdc
Copy link
Contributor

@jhefferman-sfdc jhefferman-sfdc commented Jul 31, 2025

Details

For scoped styles the scopeToken is appended as a class to the style element. This was not being done where <lwc-style> replaces itself with a <style> placeholder, which resulted in hydration failure and the <style> tag not being deduped.

Karma coverage for style dedupe has been enabled and tested and fixtures also corrected. Will add web test runner coverage in follow up PR due to time constraints and existing mismatches in web test runner.

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.

GUS work item

W-19087941

@jhefferman-sfdc jhefferman-sfdc requested a review from a team as a code owner July 31, 2025 18:37
@jhefferman-sfdc jhefferman-sfdc requested a review from wjhsf July 31, 2025 19:30
const styleId = stylesheetToId.get(stylesheet);
result += `<lwc-style style-id="lwc-style-${styleDedupePrefix}-${styleId}"></lwc-style>`;
// TODO [#2869]: `<style>`s should not have scope token classes, but required for hydration to function correctly (W-19087941).
result += `<lwc-style${hasAnyScopedStyles ? ` class="${scopeToken}"` : ''} style-id="lwc-style-${styleDedupePrefix}-${styleId}"></lwc-style>`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needs to match current non-deduped behavior here for hydration to function properly

@wjhsf
Copy link
Contributor

wjhsf commented Jul 31, 2025

👍

Main,
config.props || {},
false,
true,
Copy link
Contributor

Choose a reason for hiding this comment

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

wot's this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dedupe

@@ -0,0 +1,9 @@
export default {
test(target, snapshot, consoleCalls) {
// Expect no errors or warnings, hydration or otherwise
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Expect no errors or warnings, hydration or otherwise
// W-19087941: Expect no errors or warnings, hydration or otherwise

This is a pretty small test and it's not obvious why it exists, so we should try to keep the context around.

@jhefferman-sfdc jhefferman-sfdc merged commit 3c0b7f0 into master Aug 1, 2025
7 checks passed
@jhefferman-sfdc jhefferman-sfdc deleted the jhefferman/dedupe-coverage-scope-token-fix branch August 1, 2025 16:49
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