fix: signals in lite components (#1700)

QwikDev · Oct 11, 2022 · 799e7ec · 799e7ec
1 parent 8e23446
commit 799e7ec
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 11 deletions.
diff --git a/packages/qwik/src/core/api.md b/packages/qwik/src/core/api.md
@@ -761,7 +761,7 @@ export interface WatchCtx {
 export type WatchFn = (ctx: WatchCtx) => ValueOrPromise<void | (() => void)>;
 
 // @internal (undocumented)
-export const _wrapSignal: <T extends Record<any, any>, P extends keyof T>(obj: T, prop: P) => Signal<T[P]>;
+export const _wrapSignal: <T extends Record<any, any>, P extends keyof T>(obj: T, prop: P) => any;
 
 // (No @packageDocumentation comment for this package)
 

diff --git a/packages/qwik/src/core/object/q-object.ts b/packages/qwik/src/core/object/q-object.ts
@@ -444,7 +444,10 @@ export class SignalWrapper<T extends Record<string, any>, P extends keyof T> {
 export const _wrapSignal = <T extends Record<any, any>, P extends keyof T>(
   obj: T,
   prop: P
-): Signal<T[P]> => {
+): any => {
+  if (!isObject(obj)) {
+    return undefined;
+  }
   if (obj instanceof SignalImpl) {
     assertEqual(prop, 'value', 'Left side is a signal, prop must be value');
     return obj;
@@ -462,5 +465,9 @@ export const _wrapSignal = <T extends Record<any, any>, P extends keyof T>(
     }
     return new SignalWrapper(obj, prop);
   }
+  const immutable = (obj as any)[_IMMUTABLE]?.[prop];
+  if (isSignal(immutable)) {
+    return immutable;
+  }
   return obj[prop];
 };
diff --git a/packages/qwik/src/core/render/container.ts b/packages/qwik/src/core/render/container.ts
@@ -212,15 +212,10 @@ export class LocalSubscriptionManager {
 
   $addSub$(sub: Subscriptions) {
     const subs = this.$subs$;
-
-    const [type, group] = sub;
+    const group = sub[1];
     const key = sub[sub.length - 1] as string | undefined;
-    if (type === 0) {
-      if (
-        subs.some(([_type, _group, _key]) => _type === type && _group === group && _key === key)
-      ) {
-        return;
-      }
+    if (subs.some(([_type, _group, _key]) => _type === 0 && _group === group && _key === key)) {
+      return;
     }
     subs.push(sub);
     this.$addToGroup$(group, this);

diff --git a/starters/apps/e2e/src/components/signals/signals.tsx b/starters/apps/e2e/src/components/signals/signals.tsx
@@ -13,6 +13,12 @@ export const Signals = component$(() => {
   const ref2 = useSignal<Element>();
   const id = useSignal(0);
   const signal = useSignal('');
+  const renders = useStore(
+    {
+      count: 0,
+    },
+    { reactive: false }
+  );
   const store = useStore({
     foo: 10,
     attribute: 'even',
@@ -26,6 +32,7 @@ export const Signals = component$(() => {
     ref2.value!.setAttribute('data-set', 'ref2');
   });
 
+  renders.count++;
   return (
     <div aria-label={store.attribute}>
       <button
@@ -61,6 +68,7 @@ export const Signals = component$(() => {
       >
         Black background
       </button>
+      <div id="parent-renders">Parent renders: {renders.count}</div>
       <Child
         text="Message"
         count={store.foo}
@@ -71,6 +79,7 @@ export const Signals = component$(() => {
         id={id.value}
         styles={styles.value}
       />
+      <Issue1681 />
     </div>
   );
 });
@@ -86,12 +95,20 @@ interface ChildProps {
   styles: string;
 }
 export const Child = component$((props: ChildProps) => {
+  const renders = useStore(
+    {
+      count: 0,
+    },
+    { reactive: false }
+  );
+  renders.count++;
   return (
     <>
+      <div id="child-renders">Child renders: {renders.count}</div>
       <div id="text" ref={props.ref}>
         Text: {props.text}
       </div>
-      <div id="id">Id: {props.id}</div>
+      <Id id={props.id} />
       <div id="computed">{'computed: ' + props.signal.value}</div>
       <div id="stuff" ref={props.ref2}>
         Stuff: {props.count}
@@ -100,3 +117,26 @@ export const Child = component$((props: ChildProps) => {
     </>
   );
 });
+
+export const Id = (props: any) => <div id="id">Id: {props.id}</div>;
+
+export const C = ({ who, count }: any) => (
+  <>
+    Count {who} is {count}
+  </>
+);
+
+export const Issue1681 = component$(() => {
+  const signal = useSignal(0);
+
+  return (
+    <div>
+      <button id="issue-1681-btn" onClick$={() => signal.value++}>
+        Click
+      </button>{' '}
+      <span id="issue-1681-return">
+        <C who={'A'} count={signal.value} /> <C who={'B'} count={signal} />
+      </span>
+    </div>
+  );
+});
diff --git a/starters/e2e/e2e.signals.spec.ts b/starters/e2e/e2e.signals.spec.ts
@@ -12,13 +12,19 @@ test.describe('signals', () => {
     const incrementIdBtn = await page.locator('#increment-id');
     const backgroundBtn = await page.locator('#background');
 
+    const parentRender = await page.locator('#parent-renders');
+    const childRender = await page.locator('#child-renders');
+
     const text = await page.locator('#text');
     const id = await page.locator('#id');
     const computed = await page.locator('#computed');
     const stuff = await page.locator('#stuff');
     const body = await page.locator('body');
 
     await page.waitForTimeout(100);
+
+    await expect(parentRender).toHaveText('Parent renders: 1');
+    await expect(childRender).toHaveText('Child renders: 1');
     await expect(text).toHaveText('Text: Message');
     await expect(text).toHaveAttribute('data-set', 'ref');
     await expect(id).toHaveText('Id: 0');
@@ -27,6 +33,8 @@ test.describe('signals', () => {
     await expect(stuff).toHaveAttribute('data-set', 'ref2');
 
     await incrementBtn.click();
+    await expect(parentRender).toHaveText('Parent renders: 1');
+    await expect(childRender).toHaveText('Child renders: 1');
     await expect(text).toHaveText('Text: Message');
     await expect(text).toHaveAttribute('data-set', 'ref');
     await expect(id).toHaveText('Id: 0');
@@ -35,6 +43,8 @@ test.describe('signals', () => {
     await expect(stuff).toHaveAttribute('data-set', 'ref2');
 
     await clickBtn.click();
+    await expect(parentRender).toHaveText('Parent renders: 1');
+    await expect(childRender).toHaveText('Child renders: 2');
     await expect(text).toHaveText('Text: Message');
     await expect(text).toHaveAttribute('data-set', 'ref');
     await expect(id).toHaveText('Id: 0');
@@ -43,6 +53,8 @@ test.describe('signals', () => {
     await expect(stuff).toHaveAttribute('data-set', 'ref2');
 
     await incrementIdBtn.click();
+    await expect(parentRender).toHaveText('Parent renders: 1');
+    await expect(childRender).toHaveText('Child renders: 2');
     await expect(text).toHaveText('Text: Message');
     await expect(text).toHaveAttribute('data-set', 'ref');
     await expect(id).toHaveText('Id: 1');
@@ -52,6 +64,8 @@ test.describe('signals', () => {
     await expect(body).toHaveCSS('background-color', 'rgb(255, 255, 255)');
 
     await backgroundBtn.click();
+    await expect(parentRender).toHaveText('Parent renders: 1');
+    await expect(childRender).toHaveText('Child renders: 2');
     await expect(text).toHaveText('Text: Message');
     await expect(text).toHaveAttribute('data-set', 'ref');
     await expect(id).toHaveText('Id: 1');
@@ -60,4 +74,13 @@ test.describe('signals', () => {
     await expect(stuff).toHaveAttribute('data-set', 'ref2');
     await expect(body).toHaveCSS('background-color', 'rgb(0, 0, 0)');
   });
+
+  test('issue 1681', async ({ page }) => {
+    const result = await page.locator('#issue-1681-return');
+    const button = await page.locator('#issue-1681-btn');
+
+    await expect(result).toHaveText('Count A is 0 Count B is 0');
+    await button.click();
+    await expect(result).toHaveText('Count A is 1 Count B is 1');
+  });
 });