Skip to content

Commit

Permalink
feat(integration-karma): hydration tests base build (#2541)
Browse files Browse the repository at this point in the history 
* wip: test framework 1st checkpoint

what i don't like:
- the test (.spec) has cjs exports and needs a "def" variable, ideally, it should only need the export default and that's it.
- it modifies existing lwc plugin, because it does need to be based on the output of pepe.

wip: checkpoint 2, a lot better

wip: checkpoint 3

wip: try run it in ci

fix: ci

fix: headers check

fix: disable safari and firefox

fix: bring back safari and firefox

* wip: add hydration test commands to the readme

* test(hydration): directives

* test(hydration): component lifecycle

* test: inner-html directive

* wip: mismatch tests

* wip: rename folder

* fix: diff and reuse dom from lwc inner-html

* wip: review feedback, better errors

* refactor: use getAssociatedVM for hydration instead of getAssociatedVMIfPresent

* fix: resolve todos

* fix: display all attribute errors at once
  • Loading branch information
@jodarove
jodarove authored Oct 26, 2021
1 parent 7dc1915 commit c880cb9
Show file tree
Hide file tree
Showing 121 changed files with 1,735 additions and 115 deletions.
9 changes: 9 additions & 0 deletions .circleci/config.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ commands:
type: boolean
compat:
type: boolean
test_hydrate:
type: boolean
default: false
coverage:
type: boolean
default: true
Expand All @@ -116,6 +119,7 @@ commands:
<<# parameters.disable_synthetic >> DISABLE_SYNTHETIC=1 <</ parameters.disable_synthetic >>
<<# parameters.force_native_shadow_mode >> FORCE_NATIVE_SHADOW_MODE_FOR_TEST=1 <</ parameters.force_native_shadow_mode >>
<<# parameters.compat >> COMPAT=1 <</ parameters.compat >>
<<# parameters.test_hydrate >> TEST_HYDRATION=1 <</ parameters.test_hydrate >>
<<# parameters.coverage >> COVERAGE=1 <</ parameters.coverage >>
yarn sauce
Expand Down Expand Up @@ -190,6 +194,11 @@ jobs:
disable_synthetic: false
compat: false
force_native_shadow_mode: true
- run_karma:
disable_synthetic: true
compat: false
force_native_shadow_mode: false
test_hydrate: true
- run:
name: Compute karma coverage
command: yarn coverage
Expand Down
63 changes: 43 additions & 20 deletions packages/@lwc/engine-core/src/framework/api.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,14 @@ import {
StringReplace,
toString,
} from '@lwc/shared';
import { logError } from '../shared/logger';
import { logError, logWarn } from '../shared/logger';
import { invokeEventListener } from './invoker';
import { getVMBeingRendered } from './template';
import { EmptyArray, EmptyObject } from './utils';
import {
appendVM,
getAssociatedVMIfPresent,
getAssociatedVM,
removeVM,
rerenderVM,
runConnectedCallback,
Expand Down Expand Up @@ -70,6 +71,7 @@ import {
markAsDynamicChildren,
hydrateChildrenHook,
hydrateElmHook,
LWCDOMMode,
} from './hooks';
import { getComponentInternalDef, isComponentConstructor } from './def';
import { getUpgradableConstructor } from './upgradable-element';
Expand Down Expand Up @@ -99,7 +101,7 @@ const TextHook: Hooks<VText> = {
}

if (node.nodeValue !== vNode.text) {
logError(
logWarn(
'Hydration mismatch: text values do not match, will recover from the difference',
vNode.owner
);
Expand All @@ -126,13 +128,19 @@ const CommentHook: Hooks<VComment> = {
move: insertNodeHook, // same as insert for text nodes
remove: removeNodeHook,
hydrate: (vNode: VNode, node: Node) => {
// @todo tests.
if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line lwc-internal/no-global-node
if (node.nodeType !== Node.COMMENT_NODE) {
logError('Hydration mismatch: incorrect node type received', vNode.owner);
assert.fail('Hydration mismatch: incorrect node type received.');
}

if (node.nodeValue !== vNode.text) {
logWarn(
'Hydration mismatch: comment values do not match, will recover from the difference',
vNode.owner
);
}
}

// always set the text value to the one from the vnode.
Expand Down Expand Up @@ -180,12 +188,33 @@ const ElementHook: Hooks<VElement> = {
removeElmHook(vnode);
},
hydrate: (vnode, node) => {
vnode.elm = node as Element;

hydrateElmHook(vnode);
const elm = node as Element;
vnode.elm = elm;

const { context } = vnode.data;
const isDomManual = Boolean(context && context.lwc && context.lwc.dom === 'manual');
const isDomManual = Boolean(
!isUndefined(context) &&
!isUndefined(context.lwc) &&
context.lwc.dom === LWCDOMMode.manual
);

if (isDomManual) {
// it may be that this element has lwc:inner-html, we need to diff and in case are the same,
// remove the innerHTML from props so it reuses the existing dom elements.
const { props } = vnode.data;
if (!isUndefined(props) && !isUndefined(props.innerHTML)) {
if (elm.innerHTML === props.innerHTML) {
delete props.innerHTML;
} else {
logWarn(
`Mismatch hydrating element <${elm.tagName.toLowerCase()}>: innerHTML values do not match for element, will recover from the difference`,
vnode.owner
);
}
}
}

hydrateElmHook(vnode);

if (!isDomManual) {
hydrateChildrenHook(vnode.elm.childNodes, vnode.children, vnode.owner);
Expand Down Expand Up @@ -283,30 +312,24 @@ const CustomElementHook: Hooks<VCustomElement> = {

vnode.elm = elm as Element;

const vm = getAssociatedVMIfPresent(elm);
if (vm) {
allocateChildrenHook(vnode, vm);
}
const vm = getAssociatedVM(elm);
allocateChildrenHook(vnode, vm);

hydrateElmHook(vnode);

// Insert hook section:
if (vm) {
if (process.env.NODE_ENV !== 'production') {
assert.isTrue(vm.state === VMState.created, `${vm} cannot be recycled.`);
}
runConnectedCallback(vm);
if (process.env.NODE_ENV !== 'production') {
assert.isTrue(vm.state === VMState.created, `${vm} cannot be recycled.`);
}
runConnectedCallback(vm);

if (!(vm && vm.renderMode === RenderMode.Light)) {
if (vm.renderMode !== RenderMode.Light) {
// VM is not rendering in Light DOM, we can proceed and hydrate the slotted content.
// Note: for Light DOM, this is handled while hydrating the VM
hydrateChildrenHook(vnode.elm.childNodes, vnode.children, vm);
}

if (vm) {
hydrateVM(vm);
}
hydrateVM(vm);
},
};

Expand Down
120 changes: 62 additions & 58 deletions packages/@lwc/engine-core/src/framework/hooks.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,17 @@
* SPDX-License-Identifier: MIT
* For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT
*/
import { ArrayFilter, assert, isArray, isNull, isUndefined, noop } from '@lwc/shared';
import { EmptyArray } from './utils';
import {
ArrayFilter,
ArrayJoin,
assert,
isArray,
isNull,
isUndefined,
keys,
noop,
} from '@lwc/shared';
import { EmptyArray, parseStyleText } from './utils';
import {
createVM,
allocateInSlot,
Expand Down Expand Up @@ -45,10 +54,6 @@ function setScopeTokenClassIfNecessary(elm: Element, owner: VM) {
}
}

export function hydrateNodeHook(vNode: VNode, node: Node) {
vNode.elm = node;
}

export function updateNodeHook(oldVnode: VNode, vnode: VNode) {
const {
elm,
Expand Down Expand Up @@ -104,7 +109,7 @@ export function createElmHook(vnode: VElement) {
modComputedStyle.create(vnode);
}

const enum LWCDOMMode {
export const enum LWCDOMMode {
manual = 'manual',
}

Expand Down Expand Up @@ -258,7 +263,6 @@ export function createChildrenHook(vnode: VElement) {
}

function isElementNode(node: ChildNode): node is Element {
// @todo: should the hydrate be part of engine-dom? can we move hydrate out of the hooks?
// eslint-disable-next-line lwc-internal/no-global-node
return node.nodeType === Node.ELEMENT_NODE;
}
Expand All @@ -277,7 +281,7 @@ function vnodesAndElementHaveCompatibleAttrs(vnode: VNode, elm: Element): boolea
const elmAttrValue = renderer.getAttribute(elm, attrName);
if (attrValue !== elmAttrValue) {
logError(
`Error hydrating element: attribute "${attrName}" has different values, expected "${attrValue}" but found "${elmAttrValue}"`,
`Mismatch hydrating element <${elm.tagName.toLowerCase()}>: attribute "${attrName}" has different values, expected "${attrValue}" but found "${elmAttrValue}"`,
vnode.owner
);
nodesAreCompatible = false;
Expand All @@ -294,49 +298,41 @@ function vnodesAndElementHaveCompatibleClass(vnode: VNode, elm: Element): boolea
} = vnode;

let nodesAreCompatible = true;
let vnodeClassName;

if (!isUndefined(className) && className !== elm.className) {
// @todo: not sure if the above comparison is correct, maybe we should normalize to classlist
// className is used when class is bound to an expr.
logError(
`Mismatch hydrating element: attribute "class" has different values, expected "${className}" but found "${elm.className}"`,
vnode.owner
);
nodesAreCompatible = false;
vnodeClassName = className;
} else if (!isUndefined(classMap)) {
// classMap is used when class is set to static value.
// @todo: there might be a simpler method to do this.
const classList = renderer.getClassList(elm);
let hasClassMismatch = false;
let computedClassName = '';

// all classes from the vnode should be in the element.classList
for (const name in classMap) {
computedClassName += ' ' + name;
if (!classList.contains(name)) {
nodesAreCompatible = false;
hasClassMismatch = true;
}
}

// all classes from element.classList should be in the vnode classMap
classList.forEach((name) => {
if (!classMap[name]) {
nodesAreCompatible = false;
hasClassMismatch = true;
}
});
vnodeClassName = computedClassName.trim();

if (hasClassMismatch) {
logError(
`Mismatch hydrating element: attribute "class" has different values, expected "${computedClassName.trim()}" but found "${
elm.className
}"`,
vnode.owner
);
if (classList.length > keys(classMap).length) {
nodesAreCompatible = false;
}
}

if (!nodesAreCompatible) {
logError(
`Mismatch hydrating element <${elm.tagName.toLowerCase()}>: attribute "class" has different values, expected "${vnodeClassName}" but found "${
elm.className
}"`,
vnode.owner
);
}

return nodesAreCompatible;
}

Expand All @@ -345,39 +341,51 @@ function vnodesAndElementHaveCompatibleStyle(vnode: VNode, elm: Element): boolea
data: { style, styleDecls },
owner: { renderer },
} = vnode;
const elmStyle = renderer.getAttribute(elm, 'style');
const elmStyle = renderer.getAttribute(elm, 'style') || '';
let vnodeStyle;
let nodesAreCompatible = true;

// @todo: question: would it be the same or is there a chance that the browser tweak the result of elm.setAttribute('style', ...)?
// ex: such "str" exist that after elm.setAttribute('style', str), elm.getAttribute('style') !== str.
if (!isUndefined(style) && style !== elmStyle) {
// style is used when class is bound to an expr.
logError(
`Mismatch hydrating element: attribute "style" has different values, expected "${style}" but found "${elmStyle}".`,
vnode.owner
);
nodesAreCompatible = false;
vnodeStyle = style;
} else if (!isUndefined(styleDecls)) {
// styleMap is used when class is set to static value.
for (let i = 0; i < styleDecls.length; i++) {
const parsedVnodeStyle = parseStyleText(elmStyle);
const expectedStyle = [];
// styleMap is used when style is set to static value.
for (let i = 0, n = styleDecls.length; i < n; i++) {
const [prop, value, important] = styleDecls[i];
const elmPropValue = (elm as HTMLElement).style.getPropertyValue(prop);
const elmPropPriority = (elm as HTMLElement).style.getPropertyPriority(prop);
if (value !== elmPropValue || important !== (elmPropPriority === 'important')) {
expectedStyle.push(`${prop}: ${value + (important ? ' important!' : '')}`);

const parsedPropValue = parsedVnodeStyle[prop];

if (isUndefined(parsedPropValue)) {
nodesAreCompatible = false;
} else if (!parsedPropValue.startsWith(value)) {
nodesAreCompatible = false;
} else if (important && !parsedPropValue.endsWith('!important')) {
nodesAreCompatible = false;
}
}

// questions: is there a way to check that only those props in styleMap are set in the element?
// how to generate the style?
logError('Error hydrating element: attribute "style" has different values.', vnode.owner);
if (keys(parsedVnodeStyle).length > styleDecls.length) {
nodesAreCompatible = false;
}

vnodeStyle = ArrayJoin.call(expectedStyle, ';');
}

if (!nodesAreCompatible) {
// style is used when class is bound to an expr.
logError(
`Mismatch hydrating element <${elm.tagName.toLowerCase()}>: attribute "style" has different values, expected "${vnodeStyle}" but found "${elmStyle}".`,
vnode.owner
);
}

return nodesAreCompatible;
}

function throwHydrationError() {
// @todo: maybe create a type for these type of hydration errors
assert.fail('Server rendered elements do not match client side generated elements');
}

Expand All @@ -387,7 +395,7 @@ export function hydrateChildrenHook(elmChildren: NodeListOf<ChildNode>, children

if (elmChildren.length !== filteredVNodes.length) {
logError(
`Hydration mismatch: incorrect number of rendered elements, expected ${filteredVNodes.length} but found ${elmChildren.length}.`,
`Hydration mismatch: incorrect number of rendered nodes, expected ${filteredVNodes.length} but found ${elmChildren.length}.`,
vm
);
throwHydrationError();
Expand All @@ -405,25 +413,21 @@ export function hydrateChildrenHook(elmChildren: NodeListOf<ChildNode>, children
if (isElementNode(childNode)) {
if (ch.sel?.toLowerCase() !== childNode.tagName.toLowerCase()) {
logError(
`Hydration mismatch: expecting element with tag "${ch.sel}" but found "${childNode.tagName}".`,
`Hydration mismatch: expecting element with tag "${ch.sel?.toLowerCase()}" but found "${childNode.tagName.toLowerCase()}".`,
vm
);

throwHydrationError();
}

// Note: props are not yet set
const hasIncompatibleAttrs = vnodesAndElementHaveCompatibleAttrs(ch, childNode);
const hasIncompatibleClass = vnodesAndElementHaveCompatibleClass(ch, childNode);
const hasIncompatibleStyle = vnodesAndElementHaveCompatibleStyle(ch, childNode);
const isVNodeAndElementCompatible =
vnodesAndElementHaveCompatibleAttrs(ch, childNode) &&
vnodesAndElementHaveCompatibleClass(ch, childNode) &&
vnodesAndElementHaveCompatibleStyle(ch, childNode);
hasIncompatibleAttrs && hasIncompatibleClass && hasIncompatibleStyle;

if (!isVNodeAndElementCompatible) {
logError(
`Hydration mismatch: incompatible attributes for element with tag "${childNode.tagName}".`,
vm
);

throwHydrationError();
}
}
Expand Down
Loading

0 comments on commit c880cb9

Please sign in to comment.