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

Revert "Merge pull request #4234 from preactjs/multi-root-shared-commit" #4297

Merged
merged 2 commits into from
Mar 17, 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
5 changes: 4 additions & 1 deletion compat/test/browser/useSyncExternalStore.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -658,7 +658,10 @@ describe('useSyncExternalStore', () => {
await act(() => {
store.set(1);
});
assertLog([1, 1, 'Reset back to 0', 0, 0]);
// Preact logs differ from React here cuz of how we do rerendering. We
// rerender subtrees and then commit effects so Child2 never sees the
// update to 1 cuz Child1 rerenders and runs its layout effects first.
assertLog([1, /*1,*/ 'Reset back to 0', 0, 0]);
expect(container.textContent).to.equal('00');
});

Expand Down
29 changes: 8 additions & 21 deletions src/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { assign } from './util';
import { diff, commitRoot } from './diff/index';
import options from './options';
import { Fragment } from './create-element';
import { EMPTY_ARR, MODE_HYDRATE } from './constants';
import { MODE_HYDRATE } from './constants';

/**
* Base Component class. Provides `setState()` and `forceUpdate()`, which
Expand Down Expand Up @@ -120,10 +120,12 @@ export function getDomSibling(vnode, childIndex) {
* Trigger in-place re-rendering of a component.
* @param {Component} component The component to rerender
*/
function renderComponent(component, commitQueue, refQueue) {
function renderComponent(component) {
let oldVNode = component._vnode,
oldDom = oldVNode._dom,
parentDom = component._parentDom;
parentDom = component._parentDom,
commitQueue = [],
refQueue = [];

if (parentDom) {
const newVNode = assign({}, oldVNode);
Expand All @@ -145,14 +147,11 @@ function renderComponent(component, commitQueue, refQueue) {

newVNode._original = oldVNode._original;
newVNode._parent._children[newVNode._index] = newVNode;

newVNode._nextDom = undefined;
commitRoot(commitQueue, newVNode, refQueue);

if (newVNode._dom != oldDom) {
updateParentDomPointers(newVNode);
}

return newVNode;
}
}

Expand Down Expand Up @@ -222,33 +221,21 @@ const depthSort = (a, b) => a._vnode._depth - b._vnode._depth;
/** Flush the render queue by rerendering all queued components */
function process() {
let c;
let commitQueue = [];
let refQueue = [];
let root;
rerenderQueue.sort(depthSort);
// Don't update `renderCount` yet. Keep its value non-zero to prevent unnecessary
// process() calls from getting scheduled while `queue` is still being consumed.
while ((c = rerenderQueue.shift())) {
if (c._dirty) {
let renderQueueLength = rerenderQueue.length;
root = renderComponent(c, commitQueue, refQueue) || root;
// If this WAS the last component in the queue, run commit callbacks *before* we exit the tight loop.
// This is required in order for `componentDidMount(){this.setState()}` to be batched into one flush.
// Otherwise, also run commit callbacks if the render queue was mutated.
if (renderQueueLength === 0 || rerenderQueue.length > renderQueueLength) {
commitRoot(commitQueue, root, refQueue);
refQueue.length = commitQueue.length = 0;
root = undefined;
renderComponent(c);
if (rerenderQueue.length > renderQueueLength) {
// When i.e. rerendering a provider additional new items can be injected, we want to
// keep the order from top to bottom with those new items so we can handle them in a
// single pass
rerenderQueue.sort(depthSort);
} else if (root) {
if (options._commit) options._commit(root, EMPTY_ARR);
}
}
}
if (root) commitRoot(commitQueue, root, refQueue);
process._rerenderCount = 0;
}

Expand Down
2 changes: 2 additions & 0 deletions src/diff/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ export function diff(
* @param {VNode} root
*/
export function commitRoot(commitQueue, root, refQueue) {
root._nextDom = undefined;

Choose a reason for hiding this comment

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

why you put undefined in _nextDom ?


for (let i = 0; i < refQueue.length; i++) {
applyRef(refQueue[i], refQueue[++i], refQueue[++i]);
}
Expand Down
2 changes: 1 addition & 1 deletion src/render.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export function render(vnode, parentDom, replaceNode) {
refQueue
);

vnode._nextDom = undefined;
// Flush all queued effects
commitRoot(commitQueue, vnode, refQueue);
}

Expand Down
Loading