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.

Sign up for GitHub

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

Jump to bottom

For element nodes with a single text child, directly set textContent #3939

Closed
wants to merge 8 commits into from
Closed

For element nodes with a single text child, directly set textContent #3939

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
aa707bc
Directly set textContent when only child of an element vnode is a string
andrewiggins Mar 17, 2023
cf60098
Update dom op tracking tests
andrewiggins Mar 17, 2023
13cf8ef
Properly unmount old children when transition to and from a single te…
andrewiggins Mar 17, 2023
8b9f508
Invoke unmount lifecycles before removing DOM
andrewiggins Mar 17, 2023
2a3bba9
Improve comment
andrewiggins Mar 20, 2023
ff54c49
Merge branch 'master' into skip-append-for-element-text-child
andrewiggins Apr 3, 2023
4b5e755
Add run tracking to hydrate benchmark
andrewiggins Apr 3, 2023
2226d4c
Add comment demonstrating how to add custom tracing categories to ben…
andrewiggins Apr 4, 2023
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
14 changes: 10 additions & 4 deletions benches/src/hydrate1k.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
measureMemory,
testElementText,
afterFrame,
afterFrameAsync
afterFrameAsync,
markRunStart,
markRunEnd
} from './util.js';
import * as framework from 'framework';
import { getComponents } from '../src/keyed-children/components.js';
Expand Down Expand Up @@ -101,17 +103,20 @@
testElementText(lastRowSel, '1000');
await clickRemove(hydrateRoot, `WARMUP ${i} - prehydrate`, 1000, 1000);

// Hydrate the root
const store = new Store();
store.data = baseStore.data.slice();
markRunStart(`warmup-${i}`);
createRoot(hydrateRoot).hydrate(createElement(Main, { store }));
await markRunEnd(`warmup-${i}`);

// Verify hydrate has correct markup and is properly hydrated
testElementText(firstRowSel, '1');
testElementText(lastRowSel, '1000');
await clickRemove(hydrateRoot, `WARMUP ${i} - posthydrate`, 1000, 999);
}

function timedRun() {
async function timedRun() {
afterFrame(function () {
performance.mark('stop');
performance.measure(measureName, 'start', 'stop');
Expand All @@ -123,20 +128,21 @@
const store = new Store();
store.data = baseStore.data.slice();

markRunStart('final');
performance.mark('start');
createRoot(hydrateRoot).hydrate(createElement(Main, { store }));
await markRunEnd('final');
}

async function main() {
// const WARMUP_COUNT = 5;
const WARMUP_COUNT = 5;
for (let i = 0; i < WARMUP_COUNT; i++) {
await warmupRun(i);
}

await afterFrameAsync();

timedRun();
await timedRun();
}

initializeTemplate().then(main);
Expand Down
26 changes: 23 additions & 3 deletions src/diff/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,14 +431,34 @@ function diffElementNodes(

diffProps(dom, newProps, oldProps, isSvg, isHydrating);

// If the new vnode didn't have dangerouslySetInnerHTML, diff its children
let newChildren = newProps.children;
if (newHtml) {
newVNode._children = [];
} else if (typeof newChildren === 'string') {
if (newChildren !== oldProps.children) {
// Unmount any previous children
if (oldVNode._children) {
while ((i = oldVNode._children.pop())) {
// Setting textContent on the dom element will unmount all DOM nodes
// of the previous children, so we don't need to remove DOM in this
// call to unmount
unmount(i, oldVNode, true);
}
}

dom.textContent = newChildren;
}
} else {
i = newVNode.props.children;
// Previous render was a single text child. New children are not so let's
// unmount the previous text child
if (typeof oldProps.children === 'string') {
dom.removeChild(dom.firstChild);
}

// If the new vnode didn't have dangerouslySetInnerHTML, diff its children
diffChildren(
dom,
isArray(i) ? i : [i],
isArray(newChildren) ? newChildren : [newChildren],
newVNode,
oldVNode,
globalContext,
Expand Down
Loading