diff --git a/packages/brisa/src/utils/client-build-plugin/integration.test.tsx b/packages/brisa/src/utils/client-build-plugin/integration.test.tsx index 4fe77edb7..7aab2a7f8 100644 --- a/packages/brisa/src/utils/client-build-plugin/integration.test.tsx +++ b/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(0); + + effect(() => { + a.value++; + mockEffect(a.value); + }); + + return null; + }; + `; + + document.body.innerHTML = ''; + + 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(0); + + effect(() => { + a.value++; + a.value++; + mockEffect(a.value); + mockEffect(a.value); + }); + + return null; + }; + `; + + document.body.innerHTML = ''; + + 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(0); + const b = state(0); + + effect(() => { + b.value++; + b.value++; + mockEffect(a.value); + }); + + return null; + }; + `; + + document.body.innerHTML = ''; + + 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() { diff --git a/packages/brisa/src/utils/client-build-plugin/optimize-effects/index.test.ts b/packages/brisa/src/utils/client-build-plugin/optimize-effects/index.test.ts index 5a157590b..a3e32555f 100644 --- a/packages/brisa/src/utils/client-build-plugin/optimize-effects/index.test.ts +++ b/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(); + }); }); }); }); diff --git a/packages/brisa/src/utils/client-build-plugin/optimize-effects/index.ts b/packages/brisa/src/utils/client-build-plugin/optimize-effects/index.ts index 7bc8c7f7c..92788e6e2 100644 --- a/packages/brisa/src/utils/client-build-plugin/optimize-effects/index.ts +++ b/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 }); } diff --git a/packages/brisa/src/utils/get-client-code-in-page/index.test.ts b/packages/brisa/src/utils/get-client-code-in-page/index.test.ts index 1f44643ec..27991ba1d 100644 --- a/packages/brisa/src/utils/get-client-code-in-page/index.test.ts +++ b/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 diff --git a/packages/brisa/src/utils/signals/index.test.ts b/packages/brisa/src/utils/signals/index.test.ts index a66fd93b3..d323dd457 100644 --- a/packages/brisa/src/utils/signals/index.test.ts +++ b/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(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(0); diff --git a/packages/brisa/src/utils/signals/index.ts b/packages/brisa/src/utils/signals/index.ts index 087f93764..bd7d3e87b 100644 --- a/packages/brisa/src/utils/signals/index.ts +++ b/packages/brisa/src/utils/signals/index.ts @@ -102,8 +102,6 @@ export default function signals() { } function state(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(effects, this); - const clonedEffects = new Set([...currentEffects]); + const clonedEffects = new Set(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 diff --git a/packages/docs/building-your-application/components-details/web-components.md b/packages/docs/building-your-application/components-details/web-components.md index 348a34cc1..71f5dfa92 100644 --- a/packages/docs/building-your-application/components-details/web-components.md +++ b/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(); - 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.