diff --git a/.changeset/cuddly-roses-start.md b/.changeset/cuddly-roses-start.md new file mode 100644 index 0000000000..b0a0e49a1a --- /dev/null +++ b/.changeset/cuddly-roses-start.md @@ -0,0 +1,9 @@ +--- +"@lynx-js/web-mainthread-apis": patch +"@lynx-js/web-worker-rpc": patch +"@lynx-js/web-constants": patch +--- + +fix: when a list-item is deleted from list, the deleted list-item is still showed incorrectly. + +This is because the `enqueueComponent` method does not delete the node from the Element Tree. It is only to maintain the display node on RL, and lynx web needs to delete the dom additionally. diff --git a/packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts b/packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts index e2e0ed8db1..c839f9f4a0 100644 --- a/packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts +++ b/packages/web-platform/web-constants/src/types/MainThreadGlobalThis.ts @@ -192,9 +192,7 @@ export type UpdateListInfoAttributeValue = { insertAction: { position: number; }[]; - removeAction: { - position: number; - }[]; + removeAction: number[]; }; export type SetAttributePAPI = ( element: HTMLElement, diff --git a/packages/web-platform/web-mainthread-apis/ts/createMainThreadGlobalThis.ts b/packages/web-platform/web-mainthread-apis/ts/createMainThreadGlobalThis.ts index ac57f1ab8f..f3031dbbad 100644 --- a/packages/web-platform/web-mainthread-apis/ts/createMainThreadGlobalThis.ts +++ b/packages/web-platform/web-mainthread-apis/ts/createMainThreadGlobalThis.ts @@ -515,17 +515,32 @@ export function createMainThreadGlobalThis( const componentAtIndex = runtimeInfo.componentAtIndex; const enqueueComponent = runtimeInfo.enqueueComponent; const uniqueId = __GetElementUniqueID(element); + removeAction.forEach((position, i) => { + // remove list-item + const removedEle = element.children[position - i] as HTMLElement; + if (removedEle) { + const sign = __GetElementUniqueID(removedEle); + enqueueComponent?.(element, uniqueId, sign); + element.removeChild(removedEle); + } + }); for (const action of insertAction) { - componentAtIndex?.( + const childSign = componentAtIndex?.( element, uniqueId, action.position, 0, false, - ); - } - for (const action of removeAction) { - enqueueComponent?.(element, uniqueId, action.position); + ) as number | undefined; + if (typeof childSign === 'number') { + const childElement = lynxUniqueIdToElement[childSign]?.deref(); + if (childElement) { + const referenceNode = element.children[action.position]; + if (referenceNode !== childElement) { + element.insertBefore(childElement, referenceNode || null); + } + } + } } } }); diff --git a/packages/web-platform/web-tests/tests/main-thread-apis.test.ts b/packages/web-platform/web-tests/tests/main-thread-apis.test.ts index 432f8aff9a..35022b521e 100644 --- a/packages/web-platform/web-tests/tests/main-thread-apis.test.ts +++ b/packages/web-platform/web-tests/tests/main-thread-apis.test.ts @@ -1395,4 +1395,261 @@ test.describe('main thread api tests', () => { expect(ret.cssID).toBe('8'); expect(ret.name).toBe('name2'); }); + + test( + 'list update-list-info should insert at correct position', + async ({ page }, { title }) => { + const result = await page.evaluate(async () => { + const list = globalThis.__CreateList( + 0, + (list, listID, cellIndex, operationID, enableReuseNotification) => { + const item = globalThis.__CreateView(0); + item.setAttribute('data-content', 'B'); + globalThis.__AppendElement(list, item); + return globalThis.__GetElementUniqueID(item); + }, + (list, listID, sign) => {}, + ); + + const itemA = globalThis.__CreateView(0); + itemA.setAttribute('data-content', 'A'); + globalThis.__AppendElement(list, itemA); + + const updateInfo = { + insertAction: [ + { position: 0, type: 'type' }, + ], + removeAction: [], + }; + + globalThis.__SetAttribute(list, 'update-list-info', updateInfo); + + await new Promise(resolve => setTimeout(resolve, 0)); + + const children = list.children; + return { + count: children.length, + firstContent: children[0].getAttribute('data-content'), + secondContent: children[1].getAttribute('data-content'), + }; + }); + + expect(result.count).toBe(2); + expect(result.firstContent).toBe('B'); + expect(result.secondContent).toBe('A'); + }, + ); + + test( + 'list update-list-info should remove at correct position', + async ({ page }, { title }) => { + const result = await page.evaluate(async () => { + let recycledSign = -1; + const list = globalThis.__CreateList( + 0, + (list, listID, cellIndex, operationID, enableReuseNotification) => { + return 0; + }, + (list, listID, sign) => { + recycledSign = sign; + }, + ); + + const itemA = globalThis.__CreateView(0); + itemA.setAttribute('data-content', 'A'); + const itemB = globalThis.__CreateView(0); + itemB.setAttribute('data-content', 'B'); + const itemC = globalThis.__CreateView(0); + itemC.setAttribute('data-content', 'C'); + + globalThis.__AppendElement(list, itemA); + globalThis.__AppendElement(list, itemB); + globalThis.__AppendElement(list, itemC); + + const bUniqueId = globalThis.__GetElementUniqueID(itemB); + + const updateInfo = { + insertAction: [], + removeAction: [1], // Remove B + }; + + globalThis.__SetAttribute(list, 'update-list-info', updateInfo); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + const children = list.children; + return { + count: children.length, + firstContent: children[0].getAttribute('data-content'), + secondContent: children[1].getAttribute('data-content'), + recycledSign: recycledSign, + expectedRecycledSign: bUniqueId, + }; + }); + + expect(result.count).toBe(2); + expect(result.firstContent).toBe('A'); + expect(result.secondContent).toBe('C'); + expect(result.recycledSign).toBe(result.expectedRecycledSign); + }, + ); + + test( + 'list update-list-info should handle mixed insert and remove', + async ({ page }, { title }) => { + const result = await page.evaluate(async () => { + const list = globalThis.__CreateList( + 0, + (list, listID, cellIndex, operationID, enableReuseNotification) => { + const item = globalThis.__CreateView(0); + item.setAttribute('data-content', 'D'); + globalThis.__AppendElement(list, item); + return globalThis.__GetElementUniqueID(item); + }, + (list, listID, sign) => {}, + ); + + const itemA = globalThis.__CreateView(0); + itemA.setAttribute('data-content', 'A'); + const itemB = globalThis.__CreateView(0); + itemB.setAttribute('data-content', 'B'); + const itemC = globalThis.__CreateView(0); + itemC.setAttribute('data-content', 'C'); + + globalThis.__AppendElement(list, itemA); + globalThis.__AppendElement(list, itemB); + globalThis.__AppendElement(list, itemC); + + // Initial: [A, B, C] + // Remove 1 (B) -> [A, C] + // Insert at 0 (D) -> [D, A, C] + + const updateInfo = { + insertAction: [{ position: 0, type: 'type' }], + removeAction: [1], + }; + + globalThis.__SetAttribute(list, 'update-list-info', updateInfo); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + const children = list.children; + return { + count: children.length, + c0: children[0].getAttribute('data-content'), + c1: children[1].getAttribute('data-content'), + c2: children[2].getAttribute('data-content'), + }; + }); + + expect(result.count).toBe(3); + expect(result.c0).toBe('D'); + expect(result.c1).toBe('A'); + expect(result.c2).toBe('C'); + }, + ); + + test( + 'list update-list-info stress test with random operations', + async ({ page }, { title }) => { + const result = await page.evaluate(async () => { + // 1. Setup Initial State + const INITIAL_COUNT = 20; + const INSERT_COUNT = 10; + let uniqueCounter = INITIAL_COUNT; + const pendingInsertions: string[] = []; + + const list = globalThis.__CreateList( + 0, + (list, listID, cellIndex, operationID, enableReuseNotification) => { + const content = pendingInsertions.shift(); + const item = globalThis.__CreateView(0); + item.setAttribute('data-content', content || 'ERROR'); + globalThis.__AppendElement(list, item); + return globalThis.__GetElementUniqueID(item); + }, + (list, listID, sign) => {}, + ); + + // Populate initial list + const currentModel: string[] = []; + for (let i = 0; i < INITIAL_COUNT; i++) { + const content = i.toString(); + const item = globalThis.__CreateView(0); + item.setAttribute('data-content', content); + globalThis.__AppendElement(list, item); + currentModel.push(content); + } + + // 2. Generate Random Actions + // Phase A: Removals + // Randomly remove ~30% of items + const originalIndicesToRemove: number[] = []; + for (let i = 0; i < INITIAL_COUNT; i++) { + if (Math.random() < 0.3) { + originalIndicesToRemove.push(i); + } + } + // Remove from model (simulating the logic: removeAction indices are from original list) + // We need to remove them from currentModel. + // Since originalIndicesToRemove is sorted ascending, we can iterate from back to front to avoid index shifting issues when splicing? + // No, removeAction indices are "original indices". + // currentModel is [0, 1, 2, ...]. + // If we remove 1 and 3. + // Value '1' is at index 1. Value '3' is at index 3. + // So we just filter out values where parseInt(val) is in originalIndicesToRemove. + const modelAfterRemovals = currentModel.filter((val) => + !originalIndicesToRemove.includes(Number(val)) + ); + + // Phase B: Insertions + const insertAction: { position: number; type: string }[] = []; + const finalModel = [...modelAfterRemovals]; + + for (let i = 0; i < INSERT_COUNT; i++) { + // Random position in the CURRENT list (accumulated state) + const pos = Math.floor(Math.random() * (finalModel.length + 1)); + const content = (uniqueCounter++).toString(); + + finalModel.splice(pos, 0, content); + insertAction.push({ position: pos, type: 'stress' }); + pendingInsertions.push(content); + } + + // 3. Apply Update + const updateInfo = { + insertAction, + removeAction: originalIndicesToRemove, + }; + + globalThis.__SetAttribute(list, 'update-list-info', updateInfo); + + await new Promise((resolve) => setTimeout(resolve, 0)); + + // 4. Verification + const children = list.children; + const actualContent = Array.from(children).map((child) => + child.getAttribute('data-content') + ); + + return { + match: JSON.stringify(actualContent) === JSON.stringify(finalModel), + actual: actualContent, + expected: finalModel, + actions: { + remove: originalIndicesToRemove, + insert: insertAction, + }, + }; + }); + + expect(result.match).toBe(true); + if (!result.match) { + console.log( + 'Stress Test Failed Details:', + JSON.stringify(result, null, 2), + ); + } + }, + ); }); diff --git a/packages/web-platform/web-tests/tests/react.spec.ts b/packages/web-platform/web-tests/tests/react.spec.ts index eb094043c1..52effe4935 100644 --- a/packages/web-platform/web-tests/tests/react.spec.ts +++ b/packages/web-platform/web-tests/tests/react.spec.ts @@ -4644,6 +4644,41 @@ test.describe('reactlynx3 tests', () => { await diffScreenShot(page, elementName, title, 'scroll-to-position'); }, ); + test( + 'basic-element-list-remove-action', + async ({ page }, { title }) => { + test.skip(isSSR, 'not support on SSR'); + await goto(page, title); + + // Initial state: loading = true + // Expected: 1, 2, 3, 5 + await expect(page.locator('list-item').count()).resolves.toBe(4); + let ids = await page.locator('list-item').evaluateAll((items) => + items.map((i) => i.id) + ); + expect(ids).toEqual(['1', '2', '3', '5']); + + // First click: loading = false + // Expected: 1, 4, 5 + await page.locator('#target').click(); + await wait(1000); + await expect(page.locator('list-item').count()).resolves.toBe(3); + ids = await page.locator('list-item').evaluateAll((items) => + items.map((i) => i.id) + ); + expect(ids).toEqual(['1', '4', '5']); + + // Second click: loading = true + // Expected: 1, 2, 3, 5 + await page.locator('#target').click(); + await wait(1000); + await expect(page.locator('list-item').count()).resolves.toBe(4); + ids = await page.locator('list-item').evaluateAll((items) => + items.map((i) => i.id) + ); + expect(ids).toEqual(['1', '2', '3', '5']); + }, + ); test( 'basic-element-list-waterfall', diff --git a/packages/web-platform/web-tests/tests/react/basic-element-list-remove-action/index.css b/packages/web-platform/web-tests/tests/react/basic-element-list-remove-action/index.css new file mode 100644 index 0000000000..ad5c143f13 --- /dev/null +++ b/packages/web-platform/web-tests/tests/react/basic-element-list-remove-action/index.css @@ -0,0 +1,24 @@ +/* +// Copyright 2024 The Lynx Authors. All rights reserved. +// Licensed under the Apache License Version 2.0 that can be found in the +// LICENSE file in the root directory of this source tree. +*/ +.page { + width: 100vw; + height: 100vh; + display: flex; + flex-direction: column; +} + +list { + width: 100%; + height: 500px; +} + +list-item { + border-width: 5px; + border-color: green; + width: 100%; + height: 100px; + background-color: yellow; +} diff --git a/packages/web-platform/web-tests/tests/react/basic-element-list-remove-action/index.jsx b/packages/web-platform/web-tests/tests/react/basic-element-list-remove-action/index.jsx new file mode 100644 index 0000000000..e81f5fb91a --- /dev/null +++ b/packages/web-platform/web-tests/tests/react/basic-element-list-remove-action/index.jsx @@ -0,0 +1,64 @@ +// Copyright 2023 The Lynx Authors. All rights reserved. +// Licensed under the Apache License Version 2.0 that can be found in the +// LICENSE file in the root directory of this source tree. +import { root, useState, useEffect } from '@lynx-js/react'; +import './index.css'; + +function App() { + const [loading, setLoading] = useState(true); + const handleTap = () => setLoading(!loading); + + return ( + + + + + + + {loading + ? ( + <> + + + + + + ) + : ( + <> + + + + )} + + + + + ); +} + +root.render();