diff --git a/packages/qwik/src/core/v2/client/vnode-diff.ts b/packages/qwik/src/core/v2/client/vnode-diff.ts index 469db81a65d..f56e825f63d 100644 --- a/packages/qwik/src/core/v2/client/vnode-diff.ts +++ b/packages/qwik/src/core/v2/client/vnode-diff.ts @@ -38,7 +38,13 @@ import { } from '../shared/event-names'; import { ChoreType } from '../shared/scheduler'; import { hasClassAttr } from '../shared/scoped-styles'; -import type { HostElement, QElement2, QwikLoaderEventScope, fixMeAny } from '../shared/types'; +import type { + HostElement, + QElement2, + QwikLoaderEventScope, + fixMeAny, + qWindow, +} from '../shared/types'; import { DEBUG_TYPE, QContainerValue, VirtualType } from '../shared/types'; import type { DomContainer } from './dom-container'; import { @@ -592,6 +598,9 @@ export const vnode_diff = ( HANDLER_PREFIX + ':' + scope + ':' + eventName, value ); + if (eventName) { + registerQwikLoaderEvent(eventName); + } needsQDispatchEventPatch = true; continue; } @@ -670,10 +679,7 @@ export const vnode_diff = ( vCurrent && vnode_isElementVNode(vCurrent) && elementName === vnode_getElementName(vCurrent); const jsxKey: string | null = jsx.key; let needsQDispatchEventPatch = false; - if ( - !isSameElementName || - jsxKey !== vnode_getProp(vCurrent as ElementVNode, ELEMENT_KEY, null) - ) { + if (!isSameElementName || jsxKey !== getKey(vCurrent)) { // So we have a key and it does not match the current node. // We need to do a forward search to find it. // The complication is that once we start taking nodes out of order we can't use `vnode_getNextSibling` @@ -770,7 +776,9 @@ export const vnode_diff = ( } // register an event for qwik loader - ((globalThis as fixMeAny).qwikevents ||= []).push(eventName); + if (eventName) { + registerQwikLoaderEvent(eventName); + } }; while (srcKey !== null || dstKey !== null) { @@ -783,10 +791,11 @@ export const vnode_diff = ( // Source has more keys, so we need to remove them from destination if (dstKey && isHtmlAttributeAnEventName(dstKey)) { patchEventDispatch = true; + dstIdx++; } else { record(dstKey!, null); + dstIdx--; } - dstIdx++; // skip the destination value, we don't care about it. dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null; } else if (dstKey == null) { // Destination has more keys, so we need to insert them from source. @@ -817,23 +826,36 @@ export const vnode_diff = ( } else { record(srcKey, srcAttrs[srcIdx]); } + srcIdx++; // advance srcValue srcKey = srcIdx < srcLength ? srcAttrs[srcIdx++] : null; + // we need to increment dstIdx too, because we added destination key and value to the VNode + // and dstAttrs is a reference to the VNode + dstIdx++; + dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null; } else { // Source is missing the key, so we need to remove it from destination. if (isHtmlAttributeAnEventName(dstKey)) { patchEventDispatch = true; + dstIdx++; } else { record(dstKey!, null); + dstIdx--; } - dstIdx++; // skip the destination value, we don't care about it. dstKey = dstIdx < dstLength ? dstAttrs[dstIdx++] : null; } } return patchEventDispatch; } + function registerQwikLoaderEvent(eventName: string) { + const window = container.document.defaultView as qWindow | null; + if (window) { + (window.qwikevents ||= [] as string[]).push(eventName); + } + } + /** * Retrieve the child with the given key. * diff --git a/packages/qwik/src/core/v2/client/vnode-diff.unit.tsx b/packages/qwik/src/core/v2/client/vnode-diff.unit.tsx index 880cf9fab8b..f68caf59e41 100644 --- a/packages/qwik/src/core/v2/client/vnode-diff.unit.tsx +++ b/packages/qwik/src/core/v2/client/vnode-diff.unit.tsx @@ -127,47 +127,56 @@ describe('vNode-diff', () => { vnode_applyJournal(journal); expect(vNode).toMatchVDOM(test); }); + it('should remove extra text node', async () => { + const { vNode, vParent, document } = vnode_fromJSX( + + {'before'} + + {'after'} + + ); + const test = ( + + + + ); + vnode_diff( + { $journal$: journal, $scheduler$: scheduler, document } as any, + test, + vParent, + null + ); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + await expect(document.querySelector('test')).toMatchDOM(test); + }); + }); + + describe('attributes', () => { it('should update attributes', () => { - // here we need tu "emulate" the var props const { vNode, vParent, document } = vnode_fromJSX( _jsxSorted( - 'test', - {}, + 'span', + { + about: 'foo', + id: 'a', + 'on:click': () => null, + }, null, - [ - _jsxSorted( - 'span', - { - id: 'a', - about: 'name', - }, - null, - [], - 0, - null - ), - ], + [], 0, null ) ); const test = _jsxSorted( - 'test', - {}, + 'span', + { + about: 'bar', + id: 'b', + onClick: () => null, + }, null, - [ - _jsxSorted( - 'span', - { - class: 'B', - about: 'ABOUT', - }, - null, - [], - 0, - null - ), - ], + [], 0, null ); @@ -175,30 +184,228 @@ describe('vNode-diff', () => { vnode_applyJournal(journal); expect(vNode).toMatchVDOM(test); }); - it('should remove extra text node', async () => { + + it('should remove attributes - case 1', () => { const { vNode, vParent, document } = vnode_fromJSX( - - {'before'} - - {'after'} - + _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ) ); - const test = ( - - - + const test = _jsxSorted( + 'span', + { + about: 'name', + }, + null, + [], + 0, + null ); - vnode_diff( - { $journal$: journal, $scheduler$: scheduler, document } as any, - test, - vParent, + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should remove attributes - case 2', () => { + const { vNode, vParent, document } = vnode_fromJSX( + _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ) + ); + const test = _jsxSorted( + 'span', + { + id: 'a', + }, + null, + [], + 0, null ); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should remove attributes - case 3', () => { + const { vNode, vParent, document } = vnode_fromJSX( + _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ) + ); + const test = _jsxSorted( + 'span', + { + test: 'value', + }, + null, + [], + 0, + null + ); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should remove attributes - case 4', () => { + const { vNode, vParent, document } = vnode_fromJSX( + _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ) + ); + const test = _jsxSorted('span', {}, null, [], 0, null); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should add attributes - case 1', () => { + const { vNode, vParent, document } = vnode_fromJSX( + _jsxSorted( + 'span', + { + about: 'name', + }, + null, + [], + 0, + null + ) + ); + const test = _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should add attributes - case 2', () => { + const { vNode, vParent, document } = vnode_fromJSX( + _jsxSorted( + 'span', + { + id: 'a', + }, + null, + [], + 0, + null + ) + ); + const test = _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should add attributes - case 3', () => { + const { vNode, vParent, document } = vnode_fromJSX( + _jsxSorted( + 'span', + { + test: 'value', + }, + null, + [], + 0, + null + ) + ); + const test = _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); + vnode_applyJournal(journal); + expect(vNode).toMatchVDOM(test); + }); + + it('should add attributes - case 4', () => { + const { vNode, vParent, document } = vnode_fromJSX(_jsxSorted('span', {}, null, [], 0, null)); + const test = _jsxSorted( + 'span', + { + about: 'name', + id: 'a', + test: 'value', + }, + null, + [], + 0, + null + ); + vnode_diff({ $journal$: journal, document } as any, test, vParent, null); vnode_applyJournal(journal); expect(vNode).toMatchVDOM(test); - await expect(document.querySelector('test')).toMatchDOM(test); }); }); + describe('keys', () => { it('should not reuse element because old has a key and new one does not', () => { const { vNode, vParent, document } = vnode_fromJSX( diff --git a/packages/qwik/src/core/v2/shared/types.ts b/packages/qwik/src/core/v2/shared/types.ts index 167d602e672..2791aabd42d 100644 --- a/packages/qwik/src/core/v2/shared/types.ts +++ b/packages/qwik/src/core/v2/shared/types.ts @@ -54,6 +54,12 @@ export interface QElement2 extends HTMLElement { qDispatchEvent?: (event: Event, scope: QwikLoaderEventScope) => boolean; } +export type qWindow = Window & { + qwikevents: { + push: (...e: string[]) => void; + }; +}; + export type QwikLoaderEventScope = '-document' | '-window' | ''; export const isContainer2 = (container: any): container is Container2 => { diff --git a/packages/qwik/src/core/v2/tests/component.spec.tsx b/packages/qwik/src/core/v2/tests/component.spec.tsx index 67684029aa7..549d75de111 100644 --- a/packages/qwik/src/core/v2/tests/component.spec.tsx +++ b/packages/qwik/src/core/v2/tests/component.spec.tsx @@ -10,14 +10,14 @@ import { jsx, useComputed$, useVisibleTask$, + type JSXOutput, + useSignal, + useStore, } from '@builder.io/qwik'; -import { domRender, ssrRenderToDom } from '@builder.io/qwik/testing'; +import { domRender, ssrRenderToDom, trigger } from '@builder.io/qwik/testing'; import { describe, expect, it } from 'vitest'; -import { cleanupAttrs, trigger } from '../../../testing/element-fixture'; +import { cleanupAttrs } from '../../../testing/element-fixture'; import { ErrorProvider } from '../../../testing/rendering.unit-util'; -import type { JSXOutput } from '../../render/jsx/types/jsx-node'; -import { useSignal } from '../../use/use-signal'; -import { useStore } from '../../use/use-store.public'; import { HTML_NS, MATH_NS, SVG_NS } from '../../util/markers'; import { delay } from '../../util/promises'; diff --git a/packages/qwik/src/qwikloader.ts b/packages/qwik/src/qwikloader.ts index f0ef87f6f2c..2db4c1f78c7 100644 --- a/packages/qwik/src/qwikloader.ts +++ b/packages/qwik/src/qwikloader.ts @@ -1,7 +1,7 @@ import type { QwikSymbolEvent, QwikVisibleEvent } from './core/render/jsx/types/jsx-qwik-events'; import type { QContainerElement } from './core/container/container'; import type { QContext } from './core/state/context'; -import type { QElement2, QwikLoaderEventScope } from './core/v2/shared/types'; +import type { QElement2, QwikLoaderEventScope, qWindow } from './core/v2/shared/types'; /** * Set up event listening for browser. @@ -17,11 +17,6 @@ export const qwikLoader = ( hasInitialized?: number ) => { const Q_CONTEXT = '__q_context__'; - type qWindow = Window & { - qwikevents: { - push: (...e: string[]) => void; - }; - }; const win = window as unknown as qWindow; const events = new Set();