fix(signals): fix loop in effects

packages/brisa/src/utils/client-build-plugin/integration.test.tsx

@@ -3290,7 +3290,7 @@ describe('integration', () => {
       expect(window.mockCallback).toHaveBeenCalledTimes(1);
     });
 
-    it('should execute again the effect if is updated during effect registration', () => {
+    it('should register the sub effect after change the value', async () => {
       window.mockEffect = mock((s: string) => {});
 
       const code = `
@@ -3303,8 +3303,9 @@ describe('integration', () => {
               effect(() => mockEffect("B", b.value));
             }
             mockEffect("A", a.value);
-            a.value = 1;
           });
+
+          window.increase = () => a.value++;
   
           return null;
         };
@@ -3317,6 +3318,9 @@ describe('integration', () => {
         'src/web-components/unregister-subeffect.tsx',
       );
 
+      window.increase();
+      await Bun.sleep(0);
+
       expect(window.mockEffect).toHaveBeenCalledTimes(3);
       expect(window.mockEffect.mock.calls[0]).toEqual(['A', 0]);
       expect(window.mockEffect.mock.calls[1]).toEqual(['B', 'x']);
@@ -3366,6 +3370,82 @@ describe('integration', () => {
       expect(window.mockEffect.mock.calls[4]).toEqual(['B', 'y']);
     });
 
+    it('should not call an effect infinitelly (setter + getter inside)', () => {
+      window.mockEffect = mock((s: string) => {});
+
+      const code = `
+        export default function Component({}, { state, effect }) {
+          const a = state<number>(0);
+
+          effect(() => {
+            a.value++;
+            mockEffect(a.value);
+          });
+
+          return null;
+        };
+      `;
+
+      document.body.innerHTML = '<effect-control></effect-control>';
+
+      defineBrisaWebComponent(code, 'src/web-components/effect-control.tsx');
+
+      expect(window.mockEffect).toHaveBeenCalledTimes(1);
+      expect(window.mockEffect.mock.calls.toString()).toBe('1');
+    });
+
+    it('should not call an effect infinitelly (multi setters + getters inside)', () => {
+      window.mockEffect = mock((s: string) => {});
+
+      const code = `
+        export default function Component({}, { state, effect }) {
+          const a = state<number>(0);
+
+          effect(() => {
+            a.value++;
+            a.value++;
+            mockEffect(a.value);
+            mockEffect(a.value);
+          });
+
+          return null;
+        };
+      `;
+
+      document.body.innerHTML = '<effect-control></effect-control>';
+
+      defineBrisaWebComponent(code, 'src/web-components/effect-control.tsx');
+
+      expect(window.mockEffect).toHaveBeenCalledTimes(2);
+      expect(window.mockEffect.mock.calls.toString()).toBe('2,2');
+    });
+
+    it('should skip for infinit effect call when the setters are different than the getter', () => {
+      window.mockEffect = mock((s: string) => {});
+
+      const code = `
+        export default function Component({}, { state, effect }) {
+          const a = state<number>(0);
+          const b = state<number>(0);
+
+          effect(() => {
+            b.value++;
+            b.value++;
+            mockEffect(a.value);
+          });
+
+          return null;
+        };
+      `;
+
+      document.body.innerHTML = '<effect-control></effect-control>';
+
+      defineBrisaWebComponent(code, 'src/web-components/effect-control.tsx');
+
+      expect(window.mockEffect).toHaveBeenCalledTimes(1);
+      expect(window.mockEffect.mock.calls.toString()).toBe('0');
+    });
+
     it('should be possible to return an array and keep the reactivity', () => {
       const userInfoCode = `
           export default function UserInfo() {

packages/brisa/src/utils/client-build-plugin/optimize-effects/index.test.ts

@@ -1,9 +1,11 @@
-import { describe, expect, it } from 'bun:test';
+import { describe, expect, it, spyOn } from 'bun:test';
 import type { ESTree } from 'meriyah';
 import optimizeEffects from '.';
-import { normalizeHTML } from '@/helpers';
+import { normalizeHTML, toInline } from '@/helpers';
 import AST from '@/utils/ast';
 import getWebComponentAst from '../get-web-component-ast';
+import { getConstants } from '@/constants';
+import { boldLog } from '@/utils/log/log-color';
 
 const { parseCodeToAST, generateCodeFromAST } = AST();
 const toOutput = (code: string) => {
@@ -750,6 +752,52 @@ describe('utils', () => {
 
         expect(output).toEqual(expected);
       });
+
+      it('should warning a log when an async effect is not awaited', () => {
+        const { LOG_PREFIX } = getConstants();
+        const input = `
+          export default function Component({ propName }, { effect }) {
+            effect(async () => {
+              if (propName.value) {
+                console.log("Hello world");
+              }
+            });
+          
+            return ['span', {}, 'Hello world']
+          }
+        `;
+
+        const log = spyOn(console, 'log');
+        toOutput(input);
+        const logs = toInline(log.mock.calls.flat().join(''));
+        expect(logs).toContain(LOG_PREFIX.WARN);
+        expect(
+          toInline(logs.replaceAll(LOG_PREFIX.WARN, '')).replaceAll(' ', ''),
+        ).toBe(
+          toInline(`
+        Ops! Warning:
+        --------------------------
+        ${boldLog(`The next effect function is async without an await:`)}
+
+        effect(async () => { 
+          if (propName.value) { 
+            console.log("Hello world");
+          }
+        });
+
+        It's recommended to await the async effects to avoid registration conflicts:
+
+        await effect(async () => { 
+          if (propName.value) { 
+            console.log("Hello world");
+          }
+        });
+        --------------------------
+        Docs: https://brisa.build/building-your-application/components-details/web-components#effects-effect-method  
+        `).replaceAll(' ', ''),
+        );
+        log.mockRestore();
+      });
     });
   });
 });

packages/brisa/src/utils/client-build-plugin/optimize-effects/index.ts

@@ -1,5 +1,9 @@
+import AST from '@/utils/ast';
+import { logWarning } from '@/utils/log/log-build';
 import type { ESTree } from 'meriyah';
 
+const { generateCodeFromAST } = AST('tsx');
+
 type EffectNode = ESTree.CallExpression & { effectDeps: string[] };
 type WebContextDetails = {
   effectName: string;
@@ -102,6 +106,31 @@ export default function optimizeEffects(
     }
 
     if (node?.callee?.name === webContextDetails.effectName) {
+      const current = generateCodeFromAST(this);
+      const recommended = generateCodeFromAST({
+        ...this,
+        expression: {
+          ...this.expression,
+          argument: node,
+          type: 'AwaitExpression',
+        },
+      });
+
+      if (this?.type === 'ExpressionStatement' && node.arguments[0]?.async) {
+        logWarning(
+          [
+            'The next effect function is async without an await:',
+            '',
+            ...current.split('\n'),
+            '',
+            "It's recommended to await the async effects to avoid registration conflicts:",
+            '',
+            ...recommended.split('\n'),
+          ],
+          'Docs: https://brisa.build/building-your-application/components-details/web-components#effects-effect-method',
+        );
+      }
+
       needsToAwait ||= Boolean(node.arguments[0]?.async);
       assignRNameToNode(node, { parent: this });
     }

packages/brisa/src/utils/get-client-code-in-page/index.test.ts

@@ -22,7 +22,7 @@ const i18nCode = 3072;
 const brisaSize = 5768; // TODO: Reduce this size :/
 const webComponents = 1130;
 const unsuspenseSize = 217;
-const rpcSize = 2468; // TODO: Reduce this size
+const rpcSize = 2467; // TODO: Reduce this size
 const lazyRPCSize = 4171; // TODO: Reduce this size
 // lazyRPC is loaded after user interaction (action, link),
 // so it's not included in the initial size

packages/brisa/src/utils/signals/index.test.ts

@@ -287,27 +287,6 @@ describe('signals', () => {
     reset();
   });
 
-  it('should work without race conditions between async effects and signals', async () => {
-    const { state, effect, reset } = signals();
-    const count = state<number>(0);
-    const delay = Promise.resolve();
-    let lastSeen = -1;
-
-    effect(async () => {
-      await delay;
-      lastSeen = count.value!;
-    });
-
-    effect(() => {});
-
-    await delay;
-    expect(lastSeen).toBe(0);
-    count.value = 1;
-    await delay;
-    expect(lastSeen).toBe(1);
-    reset();
-  });
-
   it('should work with "derived" method', () => {
     const { state, derived, reset } = signals();
     const count = state<number>(0);

packages/brisa/src/utils/signals/index.ts

@@ -102,8 +102,6 @@ export default function signals() {
   }
 
   function state<T>(currentValue?: T): { value: T } {
-    let calledSameEffectOnce = false;
-
     return {
       get value() {
         if (stack[0]) {
@@ -122,14 +120,11 @@ export default function signals() {
         if (skipEffect) return;
 
         const currentEffects = getSet<Effect>(effects, this);
-        const clonedEffects = new Set<Effect>([...currentEffects]);
+        const clonedEffects = new Set<Effect>(currentEffects);
 
         for (const fn of currentEffects) {
           // Avoid calling the same effect infinitely
-          if (fn === stack[0]) {
-            if (calledSameEffectOnce) continue;
-            calledSameEffectOnce = !calledSameEffectOnce;
-          }
+          if (fn === stack[0]) continue;
 
           // When is not entering means that is a new registered effect, so it is
           // already executed. However is interesting to iterate to the updated

packages/docs/building-your-application/components-details/web-components.md

@@ -551,10 +551,10 @@ This would be an example using a prop called `foo`. The props are signals readon
 You can also use async-await in effects:
 
 ```tsx
-export default ({ foo }: { foo: string }, { state, effect }: WebContext) => {
+export default async ({ foo }: { foo: string }, { state, effect }: WebContext) => {
   const bar = state<any>();
 
-  effect(async () => {
+  await effect(async () => {
     if (foo === "bar") {
       bar.value = await fetch(/* some endpoint */).then((r) => r.json());
     }
@@ -564,6 +564,10 @@ export default ({ foo }: { foo: string }, { state, effect }: WebContext) => {
 };
 ```
 
+> [!CAUTION]
+>
+> When using `async` in an `effect`, to avoid conflicts and register the effect properly before rendering or other effects, **you have to put the `await`**. After putting the `await` you ensure that the effect has been registered correctly with the corresponding internal dependencies.
+
 ## Effect on mount (`onMount` method)
 
 The `onMount` method is triggered only once, when the component has been mounted. In the case that the component is unmounted and mounted again, it will be called again, although it would be another instance of the component starting with its initial state.