Skip to content

fix: list hydrate error #2434

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

Open
DwwWxx wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: list hydrate error #2434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 5 additions & 0 deletions .changeset/flat-ties-spend.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@lynx-js/react": patch
---

fix: When hydrate list node with `SerializedSnapshotInstance` and `BackgroundSnapshotInstance`, using `item-key` from values to diff to make sure we can get correct diff result.
209 changes: 209 additions & 0 deletions packages/react/runtime/__test__/hydrate.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,32 @@ import {
BackgroundSnapshotInstance,
hydrate,
} from '../src/snapshot';
import { SnapshotOperationParams } from '../src/lifecycle/patch/snapshotPatch';
import { __pendingListUpdates } from '../src/list/pendingListUpdates';
import { getItemKeyOf } from '../src/renderToOpcodes/hydrate';

const HOLE = null;

export function formatSnapshotPatch(patch) {
const out = [];
for (let i = 0; i < patch.length;) {
const op = patch[i];
const meta = SnapshotOperationParams[op];
if (!meta) {
out.push(`UnknownOp(${String(op)})`);
i += 1;
continue;
}
const argc = meta.params.length;
const args = patch.slice(i + 1, i + 1 + argc);
out.push(`${meta.name}(${args.map(a => JSON.stringify(a)).join(', ')})`);
i += 1 + argc;
}
return out;
}

beforeEach(() => {
__pendingListUpdates.clearAttachedLists();
backgroundSnapshotInstanceManager.clear();
backgroundSnapshotInstanceManager.nextId = 0;
snapshotInstanceManager.clear();
Expand Down Expand Up @@ -325,3 +347,190 @@ describe('dual-runtime hydrate - with slot (multi-children)', () => {
`);
});
});

describe('dual-runtime hydrate - with list', () => {
const listHolder = __SNAPSHOT__(
<list id='list'>
{HOLE}
</list>,
);
const listItem = __SNAPSHOT__(
<list-item item-key={HOLE}>
<text>Item</text>
</list-item>,
);

it('should works - list', () => {
const mtsList = new SnapshotInstance(listHolder);
mtsList.ensureElements();
const listRef = mtsList.__elements[0];

const mtsListItem0 = new SnapshotInstance(listItem);
mtsListItem0.setAttribute(0, { 'item-key': 'mts-list-item-0' });
const mtsListItem1 = new SnapshotInstance(listItem);
mtsListItem1.setAttribute(0, { 'item-key': 'mts-list-item-1' });
const mtsListItem2 = new SnapshotInstance(listItem);
mtsListItem2.setAttribute(0, { 'item-key': 'mts-list-item-2' });
mtsList.insertBefore(mtsListItem0);
mtsList.insertBefore(mtsListItem1);
mtsList.insertBefore(mtsListItem2);
__pendingListUpdates.flush();
expect(listRef).toMatchInlineSnapshot(`
<list
id="list"
update-list-info={
[
{
"insertAction": [
{
"item-key": "mts-list-item-0",
"position": 0,
"type": "__snapshot_a94a8_test_10",
},
{
"item-key": "mts-list-item-1",
"position": 1,
"type": "__snapshot_a94a8_test_10",
},
{
"item-key": "mts-list-item-2",
"position": 2,
"type": "__snapshot_a94a8_test_10",
},
],
"removeAction": [],
"updateAction": [],
},
]
}
/>
`);
const getItemKeyFromValues = (values) => {
for (let index = 0; index < values?.length; index++) {
const value = values[index];
if (value && typeof value === 'object' && !Array.isArray(value)) {
if ('item-key' in value) {
return value['item-key'] ?? undefined;
}
}
}
return undefined;
};
mtsList.childNodes.forEach((node, index) => {
const itemKey = getItemKeyFromValues(node.__values);
expect(itemKey).toBeTypeOf('string');
expect(itemKey).toBe(`mts-list-item-${index}`);
});

const btsList = new BackgroundSnapshotInstance(listHolder);
const btsListItem0 = new BackgroundSnapshotInstance(listItem);
btsListItem0.setAttribute(0, { 'item-key': 'bts-list-item-0' });
const btsListItem1 = new BackgroundSnapshotInstance(listItem);
btsListItem1.setAttribute(0, { 'item-key': 'bts-list-item-1' });
const btsListItem2 = new BackgroundSnapshotInstance(listItem);
btsListItem2.setAttribute(0, { 'item-key': 'bts-list-item-2' });
btsList.insertBefore(btsListItem0);
btsList.insertBefore(btsListItem1);
btsList.insertBefore(btsListItem2);

btsList.childNodes.forEach((node, index) => {
const itemKey = getItemKeyFromValues(node.__values);
expect(itemKey).toBeTypeOf('string');
expect(itemKey).toBe(`bts-list-item-${index}`);
});
const patches = hydrate(JSON.parse(JSON.stringify(mtsList)), btsList);
expect(patches).toMatchInlineSnapshot(`
[
2,
-1,
-2,
2,
-1,
-3,
2,
-1,
-4,
0,
"__snapshot_a94a8_test_10",
2,
4,
2,
[
{
"item-key": "bts-list-item-0",
},
],
1,
-1,
2,
undefined,
0,
"__snapshot_a94a8_test_10",
3,
4,
3,
[
{
"item-key": "bts-list-item-1",
},
],
1,
-1,
3,
undefined,
0,
"__snapshot_a94a8_test_10",
4,
4,
4,
[
{
"item-key": "bts-list-item-2",
},
],
1,
-1,
4,
undefined,
]
`);
expect(formatSnapshotPatch(patches)).toMatchInlineSnapshot(`
[
"RemoveChild(-1, -2)",
"RemoveChild(-1, -3)",
"RemoveChild(-1, -4)",
"CreateElement("__snapshot_a94a8_test_10", 2)",
"SetAttributes(2, [{"item-key":"bts-list-item-0"}])",
"InsertBefore(-1, 2, )",
"CreateElement("__snapshot_a94a8_test_10", 3)",
"SetAttributes(3, [{"item-key":"bts-list-item-1"}])",
"InsertBefore(-1, 3, )",
"CreateElement("__snapshot_a94a8_test_10", 4)",
"SetAttributes(4, [{"item-key":"bts-list-item-2"}])",
"InsertBefore(-1, 4, )",
]
`);
});
});

describe('renderToOpcodes hydrate - getItemKeyOf', () => {
it('should get item-key from __listItemPlatformInfo', () => {
expect(getItemKeyOf({ __listItemPlatformInfo: { 'item-key': 'k' } }, true)).toBe('k');
});

it('should return undefined when __listItemPlatformInfo has no item-key', () => {
expect(getItemKeyOf({ __listItemPlatformInfo: {} }, true)).toBe(undefined);
});

it('should get item-key from values when before node', () => {
expect(getItemKeyOf({ values: [null, 1, { 'item-key': 'k2' }] }, true)).toBe('k2');
});

it('should return undefined when values includes item-key undefined', () => {
expect(getItemKeyOf({ values: [{ 'item-key': undefined }] }, true)).toBe(undefined);
});

it('should get item-key from __values when after node', () => {
expect(getItemKeyOf({ __values: [{ 'item-key': 'k3' }] }, false)).toBe('k3');
});
});
39 changes: 30 additions & 9 deletions packages/react/runtime/src/renderToOpcodes/hydrate.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ import { unref } from '../snapshot/ref.js';
import type { SnapshotInstance } from '../snapshot/snapshot.js';
import { isEmptyObject } from '../utils.js';

const UNREACHABLE_ITEM_KEY_NOT_FOUND = 'UNREACHABLE_ITEM_KEY_NOT_FOUND';

export interface DiffResult<K> {
$$diff: true;
// insert No.j to new
Expand All @@ -25,7 +23,34 @@ export interface DiffResult<K> {

export interface Typed {
type: string;
// from snapshotInstance
__listItemPlatformInfo?: PlatformInfo;
// from serializeSnapshotInstance
values?: any[] | undefined;
// from backgroundSnapshotInstance
__values?: any[] | undefined;
}

export function getItemKeyOf(
node: Pick<Typed, '__listItemPlatformInfo' | '__values' | 'values'>,
isBeforeNode: boolean,
): string | undefined {
if (node?.__listItemPlatformInfo) {
// if diff list children in mts, the node has __listItemPlatformInfo, so we get item-key from it
return node?.__listItemPlatformInfo?.['item-key'] ?? undefined;
}
// if the node is the before node in diff, we get item-key from values which is passed from serializeSnapshotInstance
const valueArray = (isBeforeNode ? node?.values : node?.__values) as unknown[];
for (let index = 0; index < valueArray?.length; index++) {
const value = valueArray[index];
if (value && typeof value === 'object' && !Array.isArray(value)) {
const obj = value as Record<string, unknown>;
if ('item-key' in obj) {
return obj['item-key'] as string ?? undefined;
}
}
}
return undefined;
}

export function isEmptyDiffResult<K>(diffResult: DiffResult<K>): boolean {
Expand All @@ -39,7 +64,7 @@ export function diffArrayLepus<A extends Typed, B extends Typed>(
after: B[],
isSameType: (a: A, b: B) => boolean,
onDiffChildren: (a: A, b: B, oldIndex: number, newIndex: number) => void,
isListHasItemKey: boolean,
isListItem: boolean,
): DiffResult<B> {
let lastPlacedIndex = 0;
const result: DiffResult<B> = {
Expand All @@ -52,17 +77,13 @@ export function diffArrayLepus<A extends Typed, B extends Typed>(

for (let i = 0; i < before.length; i++) {
const node = before[i]!;
const key = isListHasItemKey
? node.__listItemPlatformInfo?.['item-key'] ?? UNREACHABLE_ITEM_KEY_NOT_FOUND
: node.type;
const key = isListItem ? (getItemKeyOf(node, true) ?? node.type) : node.type;
(beforeMap[key] ??= new Set()).add([node, i]);
}

for (let i = 0; i < after.length; i++) {
const afterNode = after[i]!;
const key = isListHasItemKey
? afterNode.__listItemPlatformInfo?.['item-key'] ?? UNREACHABLE_ITEM_KEY_NOT_FOUND
: afterNode.type;
const key = isListItem ? (getItemKeyOf(afterNode, false) ?? afterNode.type) : afterNode.type;
const beforeNodes = beforeMap[key];
let beforeNode: [A, number];

Expand Down
3 changes: 1 addition & 2 deletions packages/react/runtime/src/snapshot/backgroundSnapshot.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,8 +596,7 @@ export function hydrate(
(a, b) => {
helper(a, b);
},
// Should be `false` in hydrate as SerializedSnapshotInstance has no item-key
false,
type === DynamicPartType.ListChildren,
);
diffArrayAction(
beforeChildNodes,
Expand Down
Loading