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

test(hydration): test non-static-optimized #4657

Merged
merged 2 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/karma.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ jobs:
- run: ENABLE_SYNTHETIC_SHADOW_IN_HYDRATION=1 yarn hydration:sauce:ci
- run: NODE_ENV_FOR_TEST=production yarn hydration:sauce:ci
- run: DISABLE_NATIVE_CUSTOM_ELEMENT_LIFECYCLE=1 yarn hydration:sauce:ci
- run: DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn hydration:sauce:ci

- name: Upload coverage results
uses: actions/upload-artifact@v4
Expand Down
8 changes: 4 additions & 4 deletions packages/@lwc/engine-core/src/framework/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -858,13 +858,13 @@ function haveCompatibleStaticParts(vnode: VStatic, renderer: RendererAPI) {
// Explicitly skip hydration validation when static parts don't contain `style` or `className`.
// This means the style/class attributes are either static or don't exist on the element and
// cannot be affected by hydration.
const hasMatchingStyleAttr = shouldValidateAttr(data, 'style')
? validateStyleAttr(vnode, elm, data, renderer)
: true;
const hasMatchingClass = shouldValidateAttr(data, 'className')
nolanlawson marked this conversation as resolved.
Show resolved Hide resolved
? validateClassAttr(vnode, elm, data, renderer)
: true;
if (isFalse(hasMatchingAttrs && hasMatchingStyleAttr && hasMatchingClass)) {
const hasMatchingStyleAttr = shouldValidateAttr(data, 'style')
? validateStyleAttr(vnode, elm, data, renderer)
: true;
if (isFalse(hasMatchingAttrs && hasMatchingClass && hasMatchingStyleAttr)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need to do class first, style second to match the ordering of non-static-optimized nodes. Otherwise our tests check for the ordering of console.errors and it's different between the two.

return false;
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ const { format } = require('node:util');
const { rollup } = require('rollup');
const lwcRollupPlugin = require('@lwc/rollup-plugin');
const ssr = require('@lwc/engine-server');
const { DISABLE_STATIC_CONTENT_OPTIMIZATION } = require('../shared/options');
const Watcher = require('./Watcher');

const context = {
Expand Down Expand Up @@ -70,6 +71,7 @@ async function getCompiledModule(dirName) {
strict: true,
},
enableDynamicComponents: true,
enableStaticContentOptimization: !DISABLE_STATIC_CONTENT_OPTIMIZATION,
}),
],
cache,
Expand Down
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This test was previously split into two. Now it has an if (process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION) check instead.

Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
export default {
props: {
ssr: true,
},
clientProps: {
ssr: false,
},
snapshot(target) {
const p = target.shadowRoot.querySelector('p');
return {
p,
classes: p.className,
};
},
test(target, snapshots, consoleCalls) {
const p = target.shadowRoot.querySelector('p');

// TODO [#4656]: static optimization causes mismatches for style/class only when ordering is different
if (process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION) {
expect(p).toBe(snapshots.p);
expect(p.className).toBe(snapshots.classes);

expect(consoleCalls.error).toHaveSize(0);
} else {
expect(p).not.toBe(snapshots.p);
expect(p.className).toBe('c1 c2 c3');

TestUtils.expectConsoleCallsDev(consoleCalls, {
error: [
'Mismatch hydrating element <p>: attribute "class" has different values, expected "c1 c2 c3" but found "c3 c2 c1"',
'Hydration completed with errors.',
],
});
}
},
};

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ export default {
TestUtils.expectConsoleCallsDev(consoleCalls, {
error: [
'Mismatch hydrating element <p>: attribute "data-attrs" has different values, expected "client-attrs" but found "ssr-attrs"',
'Mismatch hydrating element <p>: attribute "style" has different values, expected "background-color: blue;" but found "background-color: red;"',
'Mismatch hydrating element <p>: attribute "class" has different values, expected "client-class" but found "ssr-class"',
'Mismatch hydrating element <p>: attribute "style" has different values, expected "background-color: blue;" but found "background-color: red;"',
'Hydration completed with errors.',
],
});
Expand Down
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ditto.

Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,24 @@ export default {
test(target, snapshots, consoleCalls) {
const p = target.shadowRoot.querySelector('p');

expect(p).not.toBe(snapshots.p);
expect(p.getAttribute('style')).toBe(
'margin: 1px; border-color: red; background-color: red;'
);
// TODO [#4656]: static optimization causes mismatches for style/class only when ordering is different
if (process.env.DISABLE_STATIC_CONTENT_OPTIMIZATION) {
expect(p).toBe(snapshots.p);
expect(p.getAttribute('style')).toBe(snapshots.style);

TestUtils.expectConsoleCallsDev(consoleCalls, {
error: [
'Mismatch hydrating element <p>: attribute "style" has different values, expected "margin: 1px; border-color: red; background-color: red;" but found "background-color: red; border-color: red; margin: 1px;"',
'Hydration completed with errors.',
],
});
expect(consoleCalls.error).toHaveSize(0);
} else {
expect(p).not.toBe(snapshots.p);
expect(p.getAttribute('style')).toBe(
'margin: 1px; border-color: red; background-color: red;'
);

TestUtils.expectConsoleCallsDev(consoleCalls, {
error: [
'Mismatch hydrating element <p>: attribute "style" has different values, expected "margin: 1px; border-color: red; background-color: red;" but found "background-color: red; border-color: red; margin: 1px;"',
'Hydration completed with errors.',
],
});
}
},
};

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Loading
Loading