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/scheduler.ts b/packages/qwik/src/core/v2/shared/scheduler.ts index 64f1ed0e652..e6e7ddb5dad 100644 --- a/packages/qwik/src/core/v2/shared/scheduler.ts +++ b/packages/qwik/src/core/v2/shared/scheduler.ts @@ -94,6 +94,7 @@ import { type ResourceDescriptor, type TaskFn, } from '../../use/use-task'; +import { logWarn } from '../../util/log'; import { isPromise, maybeThen, maybeThenPassError, safeCall } from '../../util/promises'; import type { ValueOrPromise } from '../../util/types'; import { isDomContainer } from '../client/dom-container'; @@ -220,7 +221,7 @@ export const createScheduler = ( }; chore.$promise$ = new Promise((resolve) => (chore.$resolve$ = resolve)); DEBUG && debugTrace('schedule', chore, currentChore, choreQueue); - chore = sortedInsert(choreQueue, chore, choreComparator, choreUpdate); + chore = sortedInsert(choreQueue, chore); if (!journalFlushScheduled && runLater) { // If we are not currently draining, we need to schedule a drain. journalFlushScheduled = true; @@ -246,7 +247,10 @@ export const createScheduler = ( } while (choreQueue.length) { const nextChore = choreQueue.shift()!; - const comp = choreComparator(nextChore, runUptoChore); + const comp = choreComparator(nextChore, runUptoChore, false); + if (comp === null) { + continue; + } if (comp > 0) { // we have processed all of the chores up to the given chore. break; @@ -324,25 +328,6 @@ export const createScheduler = ( } }; -export const hostElementPredicate = (aHost: HostElement, bHost: HostElement): number => { - if (aHost === bHost) { - return 0; - } else { - if (vnode_isVNode(aHost) && vnode_isVNode(bHost)) { - // we are running on the client. - return vnode_documentPosition(aHost, bHost); - } else { - // we are running on the server. - // On server we can't schedule task for a different host! - // Server is SSR, and therefore scheduling for anything but the current host - // implies that things need to be re-run nad that is not supported because of streaming. - throw new Error( - 'SERVER: during HTML streaming, it is not possible to cause a re-run of tasks on a different host' - ); - } - } -}; - const toNumber = (value: number | string): number => { return typeof value === 'number' ? value : -1; }; @@ -358,7 +343,9 @@ const choreUpdate = (existing: Chore, newChore: Chore): void => { } }; -export const choreComparator = (a: Chore, b: Chore): number => { +function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: true): number; +function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: false): number | null; +function choreComparator(a: Chore, b: Chore, shouldThrowOnHostMismatch: boolean): number | null { const macroTypeDiff = (a.$type$ & ChoreType.MACRO) - (b.$type$ & ChoreType.MACRO); if (macroTypeDiff !== 0) { return macroTypeDiff; @@ -380,9 +367,13 @@ export const choreComparator = (a: Chore, b: Chore): number => { // On server we can't schedule task for a different host! // Server is SSR, and therefore scheduling for anything but the current host // implies that things need to be re-run nad that is not supported because of streaming. - throw new Error( - 'SERVER: during HTML streaming, it is not possible to cause a re-run of tasks on a different host' - ); + const errorMessage = + 'SERVER: during HTML streaming, it is not possible to cause a re-run of tasks on a different host'; + if (shouldThrowOnHostMismatch) { + throw new Error(errorMessage); + } + logWarn(errorMessage); + return null; } } @@ -398,7 +389,7 @@ export const choreComparator = (a: Chore, b: Chore): number => { } return 0; -}; +} export const intraHostPredicate = (a: Chore, b: Chore): number => { const idxDiff = toNumber(a.$idx$) - toNumber(b.$idx$); @@ -420,11 +411,7 @@ export const intraHostPredicate = (a: Chore, b: Chore): number => { return 0; }; -function sortedFindIndex( - sortedArray: T[], - value: T, - comparator: (a: T, b: T) => number -): number { +function sortedFindIndex(sortedArray: Chore[], value: Chore): number { /// We need to ensure that the `queue` is sorted by priority. /// 1. Find a place where to insert into. let bottom = 0; @@ -432,7 +419,7 @@ function sortedFindIndex( while (bottom < top) { const middle = bottom + ((top - bottom) >> 1); const midChore = sortedArray[middle]; - const comp = comparator(value, midChore); + const comp = choreComparator(value, midChore, true); if (comp < 0) { top = middle; } else if (comp > 0) { @@ -445,22 +432,17 @@ function sortedFindIndex( return ~bottom; } -function sortedInsert( - sortedArray: T[], - value: T, - comparator: (a: T, b: T) => number, - updater?: (a: T, b: T) => void -): T { +function sortedInsert(sortedArray: Chore[], value: Chore): Chore { /// We need to ensure that the `queue` is sorted by priority. /// 1. Find a place where to insert into. - const idx = sortedFindIndex(sortedArray, value, comparator); + const idx = sortedFindIndex(sortedArray, value); if (idx < 0) { /// 2. Insert the chore into the queue. sortedArray.splice(~idx, 0, value); return value; } const existing = sortedArray[idx]; - updater && updater(existing, value); + choreUpdate(existing, value); return existing; } 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(); diff --git a/packages/qwik/src/server/prefetch-implementation.ts b/packages/qwik/src/server/prefetch-implementation.ts index 2256101b71b..224892b6677 100644 --- a/packages/qwik/src/server/prefetch-implementation.ts +++ b/packages/qwik/src/server/prefetch-implementation.ts @@ -102,14 +102,14 @@ function prefetchUrlsEvent2( if (nonce) { attrs.push('nonce', nonce); } - container.openElement('link', attrs); + container.openElement('link', null, attrs); container.closeElement(); } const scriptAttrs = ['q:type', 'prefetch-bundles']; if (nonce) { scriptAttrs.push('nonce', nonce); } - container.openElement('script', scriptAttrs); + container.openElement('script', null, scriptAttrs); container.writer.write(prefetchUrlsEventScript(prefetchResources)); container.writer.write( `;document.dispatchEvent(new CustomEvent('qprefetch', {detail:{links: [location.pathname]}}))` @@ -157,7 +157,7 @@ function linkHtmlImplementation2( } } - container.openElement('link', attributes); + container.openElement('link', null, attributes); container.closeElement(); } } @@ -228,7 +228,7 @@ function linkJsImplementation2( if (nonce) { scriptAttrs.push('nonce', nonce); } - container.openElement('script', scriptAttrs); + container.openElement('script', null, scriptAttrs); const rel = prefetchImpl.linkRel || 'prefetch'; @@ -294,7 +294,7 @@ function workerFetchImplementation2( if (nonce) { scriptAttrs.push(nonce, 'nonce'); } - container.openElement('script', scriptAttrs); + container.openElement('script', null, scriptAttrs); container.writer.write(`const u=${JSON.stringify(flattenPrefetchResources(prefetchResources))};`); container.writer.write(workerFetchScript()); diff --git a/packages/qwik/src/server/v2-ssr-container.ts b/packages/qwik/src/server/v2-ssr-container.ts index ec0474fed1d..fc22e1693e2 100644 --- a/packages/qwik/src/server/v2-ssr-container.ts +++ b/packages/qwik/src/server/v2-ssr-container.ts @@ -359,8 +359,8 @@ class SSRContainer extends _SharedContainer implements ISSRContainer { closeElement(): ValueOrPromise { const currentFrame = this.currentElementFrame!; if ( - (currentFrame.parent === null && currentFrame.tagNesting !== TagNesting.HTML) || - currentFrame.tagNesting === TagNesting.BODY + (currentFrame.parent === null && currentFrame.elementName !== 'html') || + currentFrame.elementName === 'body' ) { this.drainCleanupQueue(); this.timing.render = this.renderTimer(); diff --git a/starters/e2e/e2e.toggle.e2e.ts b/starters/e2e/e2e.toggle.e2e.ts index 34004453ddf..492cd726029 100644 --- a/starters/e2e/e2e.toggle.e2e.ts +++ b/starters/e2e/e2e.toggle.e2e.ts @@ -11,8 +11,7 @@ test.describe("toggle", () => { }); }); - // TODO(v2): fix this - test.skip("should load", async ({ page }) => { + test("should load", async ({ page }) => { const title = page.locator("h1"); const mount = page.locator("#mount"); const root = page.locator("#root"); @@ -46,7 +45,7 @@ test.describe("toggle", () => { // ToggleB await btnToggle.click(); - logsStr += "Child(1)ToggleB()"; + logsStr += "ToggleB()Child(1)"; await expect(title).toHaveText("ToggleA"); await expect(mount).toHaveText("mounted in client"); @@ -65,7 +64,7 @@ test.describe("toggle", () => { // ToggleA + increment await btnToggle.click(); await btnIncrement.click(); - logsStr += "Child(2)ToggleA()Log(3)Child(3)"; + logsStr += "ToggleA()Child(2)Log(3)Child(3)"; await expect(title).toHaveText("ToggleB"); await expect(mount).toHaveText("mounted in client");