fix: detect changing already streamed nodes

Author: Varixo

packages/qwik/src/core/client/types.ts

@@ -77,10 +77,10 @@ export const enum VNodeFlags {
   Element /* ****************** */ = 0b00_000001,
   Virtual /* ****************** */ = 0b00_000010,
   ELEMENT_OR_VIRTUAL_MASK /* ** */ = 0b00_000011,
+  Text /* ********************* */ = 0b00_000100,
   ELEMENT_OR_TEXT_MASK /* ***** */ = 0b00_000101,
   TYPE_MASK /* **************** */ = 0b00_000111,
   INFLATED_TYPE_MASK /* ******* */ = 0b00_001111,
-  Text /* ********************* */ = 0b00_000100,
   /// Extra flag which marks if a node needs to be inflated.
   Inflated /* ***************** */ = 0b00_001000,
   /// Marks if the `ensureProjectionResolved` has been called on the node.

packages/qwik/src/core/reactive-primitives/subscriber.ts

@@ -29,6 +29,6 @@ export function getSubscriber(
   return sub;
 }
 
-function isSsrNode(value: any): value is ISsrNode {
+export function isSsrNode(value: any): value is ISsrNode {
   return '__brand__' in value && value.__brand__ === 'SsrNode';
 }

packages/qwik/src/core/shared/qrl/qrl.unit.ts

@@ -156,7 +156,7 @@ describe('serialization', () => {
   });
 
   // See https://github.com/QwikDev/qwik/issues/5087#issuecomment-1707185010
-  test.skip('should parse self-reference', () => {});
+  test.todo('should parse self-reference');
 
   test('should store resolved value', async () => {
     const q = qrl(() => Promise.resolve({ hi: 'hello' }), 'hi');

packages/qwik/src/core/shared/scheduler-document-position.ts

@@ -96,11 +96,11 @@ export const ssrNodeDocumentPosition = (a: ISsrNode, b: ISsrNode): -1 | 0 | 1 =>
   let bDepth = -1;
   while (a) {
     const ssrNode = (aSsrNodePath[++aDepth] = a);
-    a = ssrNode.parentSsrNode!;
+    a = ssrNode.parent!;
   }
   while (b) {
     const ssrNode = (bSsrNodePath[++bDepth] = b);
-    b = ssrNode.parentSsrNode!;
+    b = ssrNode.parent!;
   }
 
   while (aDepth >= 0 && bDepth >= 0) {

packages/qwik/src/core/shared/scheduler.ts

@@ -105,7 +105,7 @@ import {
   type StoreTarget,
 } from '../reactive-primitives/types';
 import { triggerEffects } from '../reactive-primitives/utils';
-import type { ISsrNode } from '../ssr/ssr-types';
+import { type ISsrNode } from '../ssr/ssr-types';
 import { runResource, type ResourceDescriptor } from '../use/use-resource';
 import {
   Task,
@@ -124,9 +124,8 @@ import { isServerPlatform } from './platform/platform';
 import { type QRLInternal } from './qrl/qrl-class';
 import { isQrl } from './qrl/qrl-utils';
 import { ssrNodeDocumentPosition, vnode_documentPosition } from './scheduler-document-position';
-import type { Container, HostElement } from './types';
+import { SsrNodeFlags, type Container, type HostElement } from './types';
 import { ChoreType } from './util-chore-type';
-import { logWarn } from './utils/log';
 import { QScopedStyle } from './utils/markers';
 import { isPromise, maybeThen, retryOnPromise, safeCall } from './utils/promises';
 import { addComponentStylePrefix } from './utils/scoped-styles';
@@ -136,6 +135,8 @@ import { invoke, newInvokeContext } from '../use/use-core';
 import { findBlockingChore, findBlockingChoreForVisible } from './scheduler-rules';
 import { createNextTick } from './platform/next-tick';
 import { AsyncComputedSignalImpl } from '../reactive-primitives/impl/async-computed-signal-impl';
+import { isSsrNode } from '../reactive-primitives/subscriber';
+import { logWarn } from './utils/log';
 
 // Turn this on to get debug output of what the scheduler is doing.
 const DEBUG: boolean = false;
@@ -336,6 +337,31 @@ export const createScheduler = (
       addBlockedChore(chore, blockingChore, blockedChores);
       return chore;
     }
+    if (isServer && chore.$host$ && isSsrNode(chore.$host$)) {
+      const isUpdatable = !!(chore.$host$.flags & SsrNodeFlags.Updatable);
+
+      if (!isUpdatable) {
+        // 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.
+        const warningMessage = `A '${choreTypeToName(
+          chore.$type$
+        )}' chore was scheduled on a host element that has already been streamed to the client.
+This can lead to inconsistencies between Server-Side Rendering (SSR) and Client-Side Rendering (CSR).
+
+Problematic chore:
+  - Type: ${choreTypeToName(chore.$type$)}
+  - Host: ${chore.$host$.toString()}
+  - Nearest element location: ${chore.$host$.currentFile}
+
+This is often caused by modifying a signal in an already rendered component during SSR.`;
+        logWarn(warningMessage);
+        DEBUG &&
+          debugTrace('schedule.SKIPPED host is not updatable', chore, choreQueue, blockedChores);
+        return chore;
+      }
+    }
     chore = sortedInsert(
       choreQueue,
       chore,
@@ -710,15 +736,6 @@ export const createScheduler = (
       } else {
         assertFalse(vnode_isVNode(aHost), 'expected aHost to be SSRNode but it is a VNode');
         assertFalse(vnode_isVNode(bHost), 'expected bHost to be SSRNode but it is a VNode');
-        // 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.
-        const errorMessage = `SERVER: during HTML streaming, re-running tasks on a different host is not allowed.
-          You are attempting to change a state that has already been streamed to the client.
-          This can lead to inconsistencies between Server-Side Rendering (SSR) and Client-Side Rendering (CSR).
-          Problematic Node: ${aHost.toString()}`;
-        logWarn(errorMessage);
         const hostDiff = ssrNodeDocumentPosition(aHost as ISsrNode, bHost as ISsrNode);
         if (hostDiff !== 0) {
           return hostDiff;
@@ -851,6 +868,25 @@ export function addBlockedChore(
   blockedChores.add(blockedChore);
 }
 
+function choreTypeToName(type: ChoreType): string {
+  return (
+    (
+      {
+        [ChoreType.QRL_RESOLVE]: 'Resolve QRL',
+        [ChoreType.RUN_QRL]: 'Run QRL',
+        [ChoreType.TASK]: 'Task',
+        [ChoreType.NODE_DIFF]: 'Changes diffing',
+        [ChoreType.NODE_PROP]: 'Updating node property',
+        [ChoreType.COMPONENT]: 'Component',
+        [ChoreType.RECOMPUTE_AND_SCHEDULE_EFFECTS]: 'Signal recompute',
+        [ChoreType.VISIBLE]: 'Visible',
+        [ChoreType.CLEANUP_VISIBLE]: 'Cleanup visible',
+        [ChoreType.WAIT_FOR_QUEUE]: 'Wait for queue',
+      } as Record<ChoreType, string>
+    )[type] || 'Unknown: ' + type
+  );
+}
+
 function debugChoreTypeToString(type: ChoreType): string {
   return (
     (

packages/qwik/src/core/shared/types.ts

@@ -123,3 +123,7 @@ export type SerializationStrategy =
   // TODO: implement this in the future
   // 'auto' |
   'never' | 'always';
+
+export const enum SsrNodeFlags {
+  Updatable = 1,
+}

packages/qwik/src/core/ssr/ssr-types.ts

@@ -12,6 +12,7 @@ import type { VNodeData } from '../../server/vnode-data';
 import type { Signal } from '../reactive-primitives/signal.public';
 import type { JSXNodeInternal } from '../shared/jsx/types/jsx-node';
 import type { QRL } from '../shared/qrl/qrl.public';
+import type { SsrNodeFlags } from '../shared/types';
 import type { ResourceReturnInternal } from '../use/use-resource';
 
 export type SsrAttrKey = string;
@@ -25,12 +26,15 @@ export interface StreamWriter {
 
 export interface ISsrNode {
   id: string;
-  parentSsrNode: ISsrNode | null;
-  vnodeData?: VNodeData;
+  flags: SsrNodeFlags;
+  parent: ISsrNode | null;
+  vnodeData: VNodeData;
+  currentFile: string | null;
   setProp(name: string, value: any): void;
   getProp(name: string): any;
   removeProp(name: string): void;
   addChild(child: ISsrNode): void;
+  setTreeNonUpdatable(): void;
 }
 
 /** @internal */

packages/qwik/src/core/tests/container.spec.tsx

@@ -109,7 +109,7 @@ describe('serializer v2', () => {
 
     describe('node references', () => {
       // doesn't use the vnode so not serialized
-      it.skip('should retrieve element', async () => {
+      it('should retrieve element', async () => {
         const clientContainer = await withContainer((ssr) => {
           ssr.openElement('div', ['id', 'parent']);
           ssr.textNode('Hello');
@@ -512,7 +512,7 @@ describe('serializer v2', () => {
 
     describe('DocumentSerializer, //////', () => {
       it('should serialize and deserialize', async () => {
-        const obj = new SsrNode(null, '', -1, [], [] as any);
+        const obj = new SsrNode(null, '', -1, [], [] as any, null);
         const container = await withContainer((ssr) => ssr.addRoot(obj));
         expect(container.$getObjectById$(0)).toEqual(container.element.ownerDocument);
       });

packages/qwik/src/core/tests/ssr-render.spec.tsx

@@ -1,6 +1,14 @@
-import { Slot, useSignal } from '@qwik.dev/core';
+import {
+  createContextId,
+  Slot,
+  useContext,
+  useContextProvider,
+  useSignal,
+  useTask$,
+  type Signal,
+} from '@qwik.dev/core';
 import { ssrRenderToDom, trigger } from '@qwik.dev/core/testing';
-import { describe, expect, it } from 'vitest';
+import { describe, expect, it, vi } from 'vitest';
 import { component$ } from '../shared/component.public';
 import {
   Fragment as Component,
@@ -10,6 +18,7 @@ import {
 } from '../shared/jsx/jsx-runtime';
 import { SSRComment, SSRRaw, SSRStream, SSRStreamBlock } from '../shared/jsx/utils.public';
 import { delay } from '../shared/utils/promises';
+import * as logUtils from '../shared/utils/log';
 
 const debug = false; //true;
 Error.stackTraceLimit = 100;
@@ -241,4 +250,112 @@ describe('v2 ssr render', () => {
       );
     });
   });
+
+  it('should correctly detect node changes when node was already streamed', async () => {
+    const logWarnSpy = vi.spyOn(logUtils, 'logWarn').mockImplementation(() => {});
+
+    const rootContextId = createContextId<RootContext>('root-context');
+
+    type RootContext = {
+      isLabel: Signal<boolean>;
+      isDescription: Signal<boolean>;
+    };
+
+    const Label = component$(() => {
+      const context = useContext(rootContextId);
+
+      useTask$(() => {
+        context.isLabel.value = true;
+      });
+
+      return <div>Label</div>;
+    });
+
+    const Sibling = component$(() => {
+      const context = useContext(rootContextId);
+
+      return (
+        <>
+          <p>Does Sibling know about Label? {context.isLabel.value ? 'Yes' : 'No'}</p>
+          <p>Does Sibling know about Description? {context.isDescription.value ? 'Yes' : 'No'} </p>
+        </>
+      );
+    });
+
+    const Description = component$(() => {
+      const context = useContext(rootContextId);
+
+      useTask$(() => {
+        context.isDescription.value = true;
+      });
+
+      return <div>Description</div>;
+    });
+
+    const Cmp = component$(() => {
+      const isLabel = useSignal(false);
+      const isDescription = useSignal(false);
+
+      const context: RootContext = {
+        isLabel,
+        isDescription,
+      };
+
+      useContextProvider(rootContextId, context);
+
+      return (
+        <>
+          <Label />
+          <Sibling />
+          <Description />
+        </>
+      );
+    });
+
+    await ssrRenderToDom(<Cmp />, { debug });
+
+    expect(logWarnSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it('should correctly detect attribute changes when node was already streamed', async () => {
+    const logWarnSpy = vi.spyOn(logUtils, 'logWarn').mockImplementation(() => {});
+
+    const rootContextId = createContextId<RootContext>('root-context');
+
+    type RootContext = {
+      isLabel: Signal<boolean>;
+    };
+
+    const Label = component$(() => {
+      const context = useContext(rootContextId);
+
+      useTask$(() => {
+        context.isLabel.value = true;
+      });
+
+      return <div>Label</div>;
+    });
+
+    const Cmp = component$(() => {
+      const isLabel = useSignal(false);
+
+      const context: RootContext = {
+        isLabel,
+      };
+
+      useContextProvider(rootContextId, context);
+
+      return (
+        <>
+          <div aria-label={context.isLabel.value ? 'Yes' : 'No'}>
+            <Label />
+          </div>
+        </>
+      );
+    });
+
+    await ssrRenderToDom(<Cmp />, { debug });
+
+    expect(logWarnSpy).toHaveBeenCalledTimes(1);
+  });
 });

packages/qwik/src/core/use/use-resource.ts

@@ -7,7 +7,12 @@ import { Task, TaskFlags, cleanupTask, type DescriptorBase, type Tracker } from
 
 import type { Container, HostElement, ValueOrPromise } from '../../server/qwik-types';
 import { clearAllEffects } from '../reactive-primitives/cleanup';
-import { createStore, getStoreTarget, unwrapStore } from '../reactive-primitives/impl/store';
+import {
+  createStore,
+  forceStoreEffects,
+  getStoreTarget,
+  unwrapStore,
+} from '../reactive-primitives/impl/store';
 import type { Signal } from '../reactive-primitives/signal.public';
 import { StoreFlags } from '../reactive-primitives/types';
 import { isSignal } from '../reactive-primitives/utils';
@@ -303,18 +308,22 @@ export const runResource = <T>(
       done = true;
       if (resolved) {
         done = true;
-        resource.loading = false;
-        resource._state = 'resolved';
-        resource._resolved = value as T;
-        resource._error = undefined;
+        resourceTarget.loading = false;
+        resourceTarget._state = 'resolved';
+        resourceTarget._resolved = value as T;
+        resourceTarget._error = undefined;
         resolve(value as T);
       } else {
         done = true;
-        resource.loading = false;
-        resource._state = 'rejected';
-        resource._error = value as Error;
+        resourceTarget.loading = false;
+        resourceTarget._state = 'rejected';
+        resourceTarget._error = value as Error;
         reject(value as Error);
       }
+
+      if (!isServerPlatform()) {
+        forceStoreEffects(resource, '_state');
+      }
       return true;
     }
     return false;