fix store impl

QwikDev · Aug 6, 2024 · 227d9dd · 227d9dd
1 parent 6a6b609
commit 227d9dd
Show file tree

Hide file tree

Showing 8 changed files with 69 additions and 63 deletions.
diff --git a/packages/qwik/src/core/debug.ts b/packages/qwik/src/core/debug.ts
@@ -1,9 +1,9 @@
-import { isQrl } from "../server/prefetch-strategy";
-import { isJSXNode } from "./render/jsx/jsx-runtime";
-import { isTask } from "./use/use-task";
-import { vnode_isVNode, vnode_toString } from "./v2/client/vnode";
-import { isSignal2 } from "./v2/signal/v2-signal";
-import { isStore2 } from "./v2/signal/v2-store";
+import { isQrl } from '../server/prefetch-strategy';
+import { isJSXNode } from './render/jsx/jsx-runtime';
+import { isTask } from './use/use-task';
+import { vnode_isVNode, vnode_toString } from './v2/client/vnode';
+import { isSignal2 } from './v2/signal/v2-signal';
+import { isStore2 } from './v2/signal/v2-store';
 
 const stringifyPath: any[] = [];
 export function qwikDebugToString(value: any): any {
@@ -15,12 +15,16 @@ export function qwikDebugToString(value: any): any {
     return '"' + value + '"';
   } else if (typeof value === 'number' || typeof value === 'boolean') {
     return String(value);
+  } else if (isTask(value)) {
+    return `Task(${qwikDebugToString(value.$qrl$)})`;
+  } else if (isQrl(value)) {
+    return `Qrl(${value.$symbol$})`;
   } else if (typeof value === 'object' || typeof value === 'function') {
     if (stringifyPath.includes(value)) {
       return '*';
     }
     if (stringifyPath.length > 10) {
-      debugger;
+      // debugger;
     }
     try {
       stringifyPath.push(value);
@@ -34,10 +38,6 @@ export function qwikDebugToString(value: any): any {
         return value.toString();
       } else if (isJSXNode(value)) {
         return jsxToString(value);
-      } else if (isTask(value)) {
-        return `Task(${qwikDebugToString(value.$qrl$)})`
-      } else if (isQrl(value)) {
-        return `Qrl(${value.$symbol$})`
       }
     } finally {
       stringifyPath.pop();
@@ -46,11 +46,12 @@ export function qwikDebugToString(value: any): any {
   return value;
 }
 
-
-
 export const pad = (text: string, prefix: string) => {
-  return String(text).split('\n').map((line, idx) => (idx ? prefix : '') + line).join('\n');
-}
+  return String(text)
+    .split('\n')
+    .map((line, idx) => (idx ? prefix : '') + line)
+    .join('\n');
+};
 
 export const jsxToString = (value: any): string => {
   if (isJSXNode(value)) {
@@ -82,4 +83,4 @@ export const jsxToString = (value: any): string => {
   } else {
     return String(value);
   }
-}
+};
diff --git a/packages/qwik/src/core/use/use-signal.ts b/packages/qwik/src/core/use/use-signal.ts
@@ -1,19 +1,18 @@
 import { isQwikComponent } from '../component/component.public';
-import { _createSignal, type Signal } from '../state/signal';
 import { isFunction } from '../util/types';
-import { createSignal2 } from '../v2/signal/v2-signal.public';
+import { createSignal2, type Signal2 } from '../v2/signal/v2-signal.public';
 import { invoke } from './use-core';
 import { useSequentialScope } from './use-sequential-scope';
 
 /** @public */
 export interface UseSignal {
-  <T>(): Signal<T | undefined>;
-  <T>(value: T | (() => T)): Signal<T>;
+  <T>(): Signal2<T | undefined>;
+  <T>(value: T | (() => T)): Signal2<T>;
 }
 
 /** @public */
-export const useSignal: UseSignal = <STATE>(initialState?: STATE): Signal<STATE> => {
-  const { val, set } = useSequentialScope<Signal<STATE>>();
+export const useSignal: UseSignal = <STATE>(initialState?: STATE): Signal2<STATE> => {
+  const { val, set } = useSequentialScope<Signal2<STATE>>();
   if (val != null) {
     return val;
   }
@@ -22,6 +21,6 @@ export const useSignal: UseSignal = <STATE>(initialState?: STATE): Signal<STATE>
     isFunction(initialState) && !isQwikComponent(initialState)
       ? invoke(undefined, initialState as any)
       : initialState;
-  const signal = createSignal2(value);
+  const signal = createSignal2<STATE>(value);
   return set(signal);
 };
diff --git a/packages/qwik/src/core/use/use-task.ts b/packages/qwik/src/core/use/use-task.ts
@@ -465,7 +465,6 @@ export const useComputedQrl: ComputedQRL = <T>(qrl: QRL<ComputedFn<T>>): Signal<
   return signal;
 };
 
-
 // <docs markdown="../readme.md#useVisibleTask">
 // !!DO NOT EDIT THIS COMMENT DIRECTLY!!!
 // (edit ../readme.md#useVisibleTask instead)
@@ -531,7 +530,6 @@ export const useVisibleTaskQrl = (qrl: QRL<TaskFn>, opts?: OnVisibleTaskOptions)
   }
 };
 
-
 export type TaskDescriptor = DescriptorBase<TaskFn>;
 
 export interface ResourceDescriptor<T>

diff --git a/packages/qwik/src/core/v2/shared/scheduler.ts b/packages/qwik/src/core/v2/shared/scheduler.ts
@@ -103,7 +103,7 @@ import { EffectSubscriptionsProp, isSignal2, type EffectSubscriptions } from '..
 import { serializeAttribute } from '../../render/execute-component';
 
 // Turn this on to get debug output of what the scheduler is doing.
-const DEBUG: boolean = true;
+const DEBUG: boolean = false;
 
 export const enum ChoreType {
   /// MASKS defining three levels of sorting

diff --git a/packages/qwik/src/core/v2/shared/shared-serialization.ts b/packages/qwik/src/core/v2/shared/shared-serialization.ts
@@ -21,11 +21,7 @@ import {
   isPropsProxy,
 } from '../../render/jsx/jsx-runtime';
 import { Slot } from '../../render/jsx/slot.public';
-import {
-  fastSkipSerialize,
-  getProxyFlags,
-  getSubscriptionManager
-} from '../../state/common';
+import { fastSkipSerialize, getProxyFlags, getSubscriptionManager } from '../../state/common';
 import { _CONST_PROPS, _VAR_PROPS } from '../../state/constants';
 import { Task, isTask, type ResourceReturnInternal } from '../../use/use-task';
 import { EMPTY_OBJ } from '../../util/flyweight';
@@ -42,7 +38,13 @@ import {
   Signal2,
   type EffectSubscriptions,
 } from '../signal/v2-signal';
-import { Store2, createStore2, getStoreHandler2, unwrapStore2, type StoreHandler } from '../signal/v2-store';
+import {
+  Store2,
+  createStore2,
+  getStoreHandler2,
+  unwrapStore2,
+  type StoreHandler,
+} from '../signal/v2-store';
 import type { SymbolToChunkResolver } from '../ssr/ssr-types';
 import type { fixMeAny } from './types';
 
@@ -277,7 +279,7 @@ const inflate = (container: DomContainer, target: any, needsInflationData: strin
           const idx = effect.indexOf(';');
           const prop = effect.substring(0, idx);
           const effectStr = effect.substring(idx + 1);
-          deserializeSignal2Effect(0, effectStr.split(';'), container, effects[prop] = [])
+          deserializeSignal2Effect(0, effectStr.split(';'), container, (effects[prop] = []));
         }
       }
       break;
@@ -826,14 +828,18 @@ function serialize(serializationContext: SerializationContext): void {
       const varId = $addRoot$(varProps);
       const constProps = value[_CONST_PROPS];
       const constId = $addRoot$(constProps);
-      writeString(SerializationConstant.PropsProxy_CHAR + varId + '|' + constId);
+      writeString(SerializationConstant.PropsProxy_CHAR + varId + ' ' + constId);
     } else if ((storeHandler = getStoreHandler2(value))) {
-      let store = SerializationConstant.Store_CHAR + $addRoot$(storeHandler.$target$) + ' ' + storeHandler.$flags$;
+      let store =
+        SerializationConstant.Store_CHAR +
+        $addRoot$(storeHandler.$target$) +
+        ' ' +
+        storeHandler.$flags$;
       const effects = storeHandler.$effects$;
       if (effects) {
         let sep = ' ';
         for (const propName in effects) {
-          store += sep + propName + serializeEffectSubs($addRoot$, effects[propName])
+          store += sep + propName + serializeEffectSubs($addRoot$, effects[propName]);
           sep = '|';
         }
       }
@@ -1109,7 +1115,12 @@ function deserializeSignal2(
   }
 }
 
-function deserializeSignal2Effect(idx: number, parts: string[], container: DomContainer, effects: EffectSubscriptions[]) {
+function deserializeSignal2Effect(
+  idx: number,
+  parts: string[],
+  container: DomContainer,
+  effects: EffectSubscriptions[]
+) {
   while (idx < parts.length) {
     // idx == 1 is the attribute name
     const effect = parts[idx++]

diff --git a/packages/qwik/src/core/v2/signal/v2-signal.ts b/packages/qwik/src/core/v2/signal/v2-signal.ts
@@ -227,8 +227,6 @@ export const ensureContainsEffect = (array: EffectSubscriptions[], effect: Effec
       return;
     }
   }
-  console.log('array', array);
-  console.log('array.push', array.push);
   array.push(effect);
 };
 

diff --git a/packages/qwik/src/core/v2/signal/v2-store.ts b/packages/qwik/src/core/v2/signal/v2-store.ts
@@ -1,7 +1,7 @@
 import { pad, qwikDebugToString } from '../../debug';
 import { assertDefined, assertTrue } from '../../error/assert';
 import { tryGetInvokeContext } from '../../use/use-core';
-import { isObject, isSerializableObject } from '../../util/types';
+import { isSerializableObject } from '../../util/types';
 import type { VNode } from '../client/types';
 import type { Container2, fixMeAny } from '../shared/types';
 import {
@@ -12,7 +12,7 @@ import {
   type EffectSubscriptions,
 } from './v2-signal';
 
-const DEBUG = true;
+const DEBUG = false;
 
 // eslint-disable-next-line no-console
 const log = (...args: any[]) => console.log('STORE', ...args.map(qwikDebugToString));
@@ -36,7 +36,7 @@ let _lastHandler: undefined | StoreHandler<any>;
 export const getStoreHandler2 = <T extends object>(value: T): StoreHandler<T> | null => {
   _lastHandler = undefined as any;
   return typeof value === 'object' && value && STORE in value // this implicitly sets the `_lastHandler` as a side effect.
-    ? (_lastHandler!)
+    ? _lastHandler!
     : null;
 };
 
@@ -53,11 +53,12 @@ export const isStore2 = <T extends object>(value: T): value is Store2<T> => {
   return value instanceof Store;
 };
 
-export function createStore2<T extends object>(container: Container2 | null | undefined, obj: T & Record<string, unknown>, flags: Store2Flags) {
-  return new Proxy(
-    new Store(),
-    new StoreHandler<T>(obj, flags, container || null)
-  ) as Store2<T>;
+export function createStore2<T extends object>(
+  container: Container2 | null | undefined,
+  obj: T & Record<string, unknown>,
+  flags: Store2Flags
+) {
+  return new Proxy(new Store(), new StoreHandler<T>(obj, flags, container || null)) as Store2<T>;
 }
 
 export const getOrCreateStore2 = <T extends object>(
@@ -69,11 +70,11 @@ export const getOrCreateStore2 = <T extends object>(
     let store: Store2<T> | undefined = storeWeakMap.get(obj) as Store2<T> | undefined;
     if (!store) {
       store = createStore2<T>(container, obj, flags);
-      storeWeakMap.set(obj, store as any);
+      storeWeakMap.set(obj, store);
     }
     return store as Store2<T>;
   }
-  return obj as any;
+  return obj as Store2<T>;
 };
 
 class Store {
@@ -90,7 +91,7 @@ export class StoreHandler<T extends Record<string | symbol, any>> implements Pro
     public $target$: T,
     public $flags$: Store2Flags,
     public $container$: Container2 | null
-  ) { }
+  ) {}
 
   toString() {
     const flags = [];
@@ -158,6 +159,7 @@ export class StoreHandler<T extends Record<string | symbol, any>> implements Pro
     const flags = this.$flags$;
     if (flags & Store2Flags.RECURSIVE && typeof value === 'object' && value !== null) {
       value = getOrCreateStore2(value, this.$flags$, this.$container$);
+      (target as Record<string | symbol, any>)[p] = value;
     }
     return value;
   }
@@ -168,7 +170,7 @@ export class StoreHandler<T extends Record<string | symbol, any>> implements Pro
     if (value !== oldValue) {
       DEBUG && log('Signal.set', oldValue, '->', value, pad('\n' + this.toString(), '  '));
       (target as any)[p] = value;
-      triggerEffects(this.$container$, this, this.$effects$?.[String(p)]);
+      triggerEffects(this.$container$, this, this.$effects$ ? this.$effects$[String(p)] : null);
     }
     return true;
   }

diff --git a/packages/qwik/src/core/v2/tests/use-store.spec.tsx b/packages/qwik/src/core/v2/tests/use-store.spec.tsx
@@ -9,12 +9,12 @@ import { untrack } from '../../use/use-core';
 import { useSignal } from '../../use/use-signal';
 import { useStore } from '../../use/use-store.public';
 
-const debug = true; //true;
+const debug = false; //true;
 Error.stackTraceLimit = 100;
 
 describe.each([
   { render: ssrRenderToDom }, //
-  // { render: domRender }, //
+  { render: domRender }, //
 ])('$render.name: useStore', ({ render }) => {
   it('should render value', async () => {
     const Cmp = component$(() => {
@@ -60,7 +60,7 @@ describe.each([
       </Component>
     );
   });
-  it.only('should update deep value', async () => {
+  it('should update deep value', async () => {
     const Counter = component$(() => {
       const count = useStore({ obj: { count: 123 } });
       return <button onClick$={() => count.obj.count++}>Count: {count.obj.count}!</button>;
@@ -189,9 +189,7 @@ describe.each([
         const count = useStore<any>({ jsx: 'initial' });
         log.push('Counter: ' + untrack(() => count.jsx));
         return (
-          <button
-            onClick$={() => (count.jsx = typeof count.jsx == 'string' ? <b>JSX</b> : 'text')}
-          >
+          <button onClick$={() => (count.jsx = typeof count.jsx == 'string' ? <b>JSX</b> : 'text')}>
             -{count.jsx}-
           </button>
         );
@@ -460,8 +458,7 @@ describe.each([
       );
     });
 
-    // TODO(optimizer-test): this is failing also in v1
-    it.skip('#5017 - should update child nodes for direct array', async () => {
+    it('#5017 - should update child nodes for direct array', async () => {
       const Child = component$<{ columns: string }>(({ columns }) => {
         return <div>Child: {columns}</div>;
       });
@@ -488,13 +485,13 @@ describe.each([
             <Component>
               <div>
                 {'Child: '}
-                {'INITIAL'}
+                <Signal>{'INITIAL'}</Signal>
               </div>
             </Component>
             <Component>
               <div>
                 {'Child: '}
-                {'INITIAL'}
+                <Signal>{'INITIAL'}</Signal>
               </div>
             </Component>
           </Fragment>
@@ -508,13 +505,13 @@ describe.each([
             <Component>
               <div>
                 {'Child: '}
-                {'UPDATE'}
+                <Signal>{'UPDATE'}</Signal>
               </div>
             </Component>
             <Component>
               <div>
                 {'Child: '}
-                {'UPDATE'}
+                <Signal>{'UPDATE'}</Signal>
               </div>
             </Component>
           </Fragment>
-Original file line number
+Diff line change
@@ Expand Up @@
           return;
         }
       }
-      console.log('array', array);
-      console.log('array.push', array.push);
       array.push(effect);
     };
@@ Expand Down @@