diff --git a/package.json b/package.json index 6d271d5a7..d922a0cef 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "license": "MIT", "devDependencies": { "@babel/core": "^7.22.8", + "@babel/plugin-proposal-explicit-resource-management": "^7.22.6", "@babel/plugin-syntax-jsx": "^7.21.4", "@babel/plugin-transform-modules-commonjs": "^7.22.5", "@babel/plugin-transform-react-jsx": "^7.21.4", diff --git a/packages/react-transform/src/index.ts b/packages/react-transform/src/index.ts index 3b9064394..812e41c2c 100644 --- a/packages/react-transform/src/index.ts +++ b/packages/react-transform/src/index.ts @@ -173,11 +173,11 @@ function isValueMemberExpression( ); } -const tryCatchTemplate = template.statements`var STOP_TRACKING_IDENTIFIER = HOOK_IDENTIFIER(); +const tryCatchTemplate = template.statements`var STORE_IDENTIFIER = HOOK_IDENTIFIER(); try { BODY } finally { - STOP_TRACKING_IDENTIFIER(); + STORE_IDENTIFIER.finishEffect(); }`; function wrapInTryFinally( @@ -191,7 +191,7 @@ function wrapInTryFinally( const newFunction = t.cloneNode(path.node); newFunction.body = t.blockStatement( tryCatchTemplate({ - STOP_TRACKING_IDENTIFIER: stopTrackingIdentifier, + STORE_IDENTIFIER: stopTrackingIdentifier, HOOK_IDENTIFIER: get(state, getHookIdentifier)(), BODY: t.isBlockStatement(path.node.body) ? path.node.body.body // TODO: Is it okay to elide the block statement here? diff --git a/packages/react-transform/test/browser/e2e.test.tsx b/packages/react-transform/test/browser/e2e.test.tsx index 40bfe1371..3db87a3c4 100644 --- a/packages/react-transform/test/browser/e2e.test.tsx +++ b/packages/react-transform/test/browser/e2e.test.tsx @@ -212,4 +212,27 @@ describe("React Signals babel transfrom - browser E2E tests", () => { }); expect(scratch.innerHTML).to.equal("
Hello Jane
"); }); + + it("works with the `using` keyword", async () => { + const { App } = await createComponent( + ` + import { useSignals } from "@preact/signals-react/runtime"; + + export function App({ name }) { + using _ = useSignals(); + return
Hello {name.value}
; + }`, + // Disable our babel plugin for this example so the explicit resource management plugin handles this case + { mode: "manual" } + ); + + const name = signal("John"); + await render(); + expect(scratch.innerHTML).to.equal("
Hello John
"); + + await act(() => { + name.value = "Jane"; + }); + expect(scratch.innerHTML).to.equal("
Hello Jane
"); + }); }); diff --git a/packages/react/runtime/src/auto.ts b/packages/react/runtime/src/auto.ts index 62eba4919..25be58a63 100644 --- a/packages/react/runtime/src/auto.ts +++ b/packages/react/runtime/src/auto.ts @@ -6,7 +6,7 @@ import { import React from "react"; import jsxRuntime from "react/jsx-runtime"; import jsxRuntimeDev from "react/jsx-dev-runtime"; -import { useSignals, wrapJsx } from "./index"; +import { EffectStore, useSignals, wrapJsx } from "./index"; export interface ReactDispatcher { useRef: typeof React.useRef; @@ -103,50 +103,50 @@ import { createMachine } from "xstate"; // if it doesn't signal a change in the component rendering lifecyle (NOOP). const dispatcherMachinePROD = createMachine({ - id: "ReactCurrentDispatcher_PROD", - initial: "null", - states: { - null: { - on: { - pushDispatcher: "ContextOnlyDispatcher", - }, - }, - ContextOnlyDispatcher: { - on: { - renderWithHooks_Mount_ENTER: "HooksDispatcherOnMount", - renderWithHooks_Update_ENTER: "HooksDispatcherOnUpdate", - pushDispatcher_NOOP: "ContextOnlyDispatcher", - popDispatcher_NOOP: "ContextOnlyDispatcher", - }, - }, - HooksDispatcherOnMount: { - on: { - renderWithHooksAgain_ENTER: "HooksDispatcherOnRerender", - resetHooksAfterThrow_EXIT: "ContextOnlyDispatcher", - finishRenderingHooks_EXIT: "ContextOnlyDispatcher", - }, - }, - HooksDispatcherOnUpdate: { - on: { - renderWithHooksAgain_ENTER: "HooksDispatcherOnRerender", - resetHooksAfterThrow_EXIT: "ContextOnlyDispatcher", - finishRenderingHooks_EXIT: "ContextOnlyDispatcher", - use_ResumeSuspensedMount_NOOP: "HooksDispatcherOnMount", - }, - }, - HooksDispatcherOnRerender: { - on: { - renderWithHooksAgain_ENTER: "HooksDispatcherOnRerender", - resetHooksAfterThrow_EXIT: "ContextOnlyDispatcher", - finishRenderingHooks_EXIT: "ContextOnlyDispatcher", - }, - }, - }, + id: "ReactCurrentDispatcher_PROD", + initial: "null", + states: { + null: { + on: { + pushDispatcher: "ContextOnlyDispatcher", + }, + }, + ContextOnlyDispatcher: { + on: { + renderWithHooks_Mount_ENTER: "HooksDispatcherOnMount", + renderWithHooks_Update_ENTER: "HooksDispatcherOnUpdate", + pushDispatcher_NOOP: "ContextOnlyDispatcher", + popDispatcher_NOOP: "ContextOnlyDispatcher", + }, + }, + HooksDispatcherOnMount: { + on: { + renderWithHooksAgain_ENTER: "HooksDispatcherOnRerender", + resetHooksAfterThrow_EXIT: "ContextOnlyDispatcher", + finishRenderingHooks_EXIT: "ContextOnlyDispatcher", + }, + }, + HooksDispatcherOnUpdate: { + on: { + renderWithHooksAgain_ENTER: "HooksDispatcherOnRerender", + resetHooksAfterThrow_EXIT: "ContextOnlyDispatcher", + finishRenderingHooks_EXIT: "ContextOnlyDispatcher", + use_ResumeSuspensedMount_NOOP: "HooksDispatcherOnMount", + }, + }, + HooksDispatcherOnRerender: { + on: { + renderWithHooksAgain_ENTER: "HooksDispatcherOnRerender", + resetHooksAfterThrow_EXIT: "ContextOnlyDispatcher", + finishRenderingHooks_EXIT: "ContextOnlyDispatcher", + }, + }, + }, }); ``` */ -let stopTracking: (() => void) | null = null; +let store: EffectStore | null = null; let lock = false; let currentDispatcher: ReactDispatcher | null = null; @@ -171,12 +171,12 @@ function installCurrentDispatcherHook() { isEnteringComponentRender(currentDispatcherType, nextDispatcherType) ) { lock = true; - stopTracking = useSignals(); + store = useSignals(); lock = false; } else if ( isExitingComponentRender(currentDispatcherType, nextDispatcherType) ) { - stopTracking?.(); + store?.finishEffect(); } }, }); @@ -325,7 +325,7 @@ function isExitingComponentRender( ): boolean { return Boolean( currentDispatcherType & BrowserClientDispatcherType && - nextDispatcherType & ContextOnlyDispatcherType + nextDispatcherType & ContextOnlyDispatcherType ); } diff --git a/packages/react/runtime/src/index.ts b/packages/react/runtime/src/index.ts index 754b65862..5ba2b15b9 100644 --- a/packages/react/runtime/src/index.ts +++ b/packages/react/runtime/src/index.ts @@ -24,6 +24,8 @@ export function wrapJsx(jsx: T): T { } as any as T; } +const symDispose: unique symbol = (Symbol as any).dispose || Symbol.for("Symbol.dispose"); + interface Effect { _sources: object | undefined; _start(): () => void; @@ -31,12 +33,25 @@ interface Effect { _dispose(): void; } -interface EffectStore { - updater: Effect; +export interface EffectStore { + effect: Effect; subscribe(onStoreChange: () => void): () => void; getSnapshot(): number; + finishEffect(): void; + [symDispose](): void; } +let finishUpdate: (() => void) | undefined; + +function setCurrentStore(store?: EffectStore) { + // end tracking for the current update: + if (finishUpdate) finishUpdate(); + // start tracking the new update: + finishUpdate = store && store.effect._start(); +} + +const clearCurrentStore = () => setCurrentStore(); + /** * A redux-like store whose store value is a positive 32bit integer (a 'version'). * @@ -51,20 +66,20 @@ interface EffectStore { * @see https://github.com/reactjs/rfcs/blob/main/text/0214-use-sync-external-store.md */ function createEffectStore(): EffectStore { - let updater!: Effect; + let effectInstance!: Effect; let version = 0; let onChangeNotifyReact: (() => void) | undefined; let unsubscribe = effect(function (this: Effect) { - updater = this; + effectInstance = this; }); - updater._callback = function () { + effectInstance._callback = function () { version = (version + 1) | 0; if (onChangeNotifyReact) onChangeNotifyReact(); }; return { - updater, + effect: effectInstance, subscribe(onStoreChange) { onChangeNotifyReact = onStoreChange; @@ -87,32 +102,28 @@ function createEffectStore(): EffectStore { getSnapshot() { return version; }, + finishEffect() { + clearCurrentStore(); + }, + [symDispose]() { + clearCurrentStore(); + } }; } let finalCleanup: Promise | undefined; -let finishUpdate: (() => void) | undefined; - -function setCurrentUpdater(updater?: Effect) { - // end tracking for the current update: - if (finishUpdate) finishUpdate(); - // start tracking the new update: - finishUpdate = updater && updater._start(); -} - const _queueMicroTask = Promise.prototype.then.bind(Promise.resolve()); -const clearCurrentUpdater = () => setCurrentUpdater(); /** * Custom hook to create the effect to track signals used during render and * subscribe to changes to rerender the component when the signals change. */ -export function useSignals(): () => void { - clearCurrentUpdater(); +export function useSignals(): EffectStore { + clearCurrentStore(); if (!finalCleanup) { finalCleanup = _queueMicroTask(() => { finalCleanup = undefined; - clearCurrentUpdater(); + clearCurrentStore(); }); } @@ -123,9 +134,9 @@ export function useSignals(): () => void { const store = storeRef.current; useSyncExternalStore(store.subscribe, store.getSnapshot, store.getSnapshot); - setCurrentUpdater(store.updater); + setCurrentStore(store); - return clearCurrentUpdater; + return store; } /** diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8d04d7e8c..c87c4f24c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -13,6 +13,7 @@ importers: .: specifiers: '@babel/core': ^7.22.8 + '@babel/plugin-proposal-explicit-resource-management': ^7.22.6 '@babel/plugin-syntax-jsx': ^7.21.4 '@babel/plugin-transform-modules-commonjs': ^7.22.5 '@babel/plugin-transform-react-jsx': ^7.21.4 @@ -60,6 +61,7 @@ importers: typescript: ^4.7.4 devDependencies: '@babel/core': 7.22.8 + '@babel/plugin-proposal-explicit-resource-management': 7.22.6_@babel+core@7.22.8 '@babel/plugin-syntax-jsx': 7.22.5_@babel+core@7.22.8 '@babel/plugin-transform-modules-commonjs': 7.22.5_@babel+core@7.22.8 '@babel/plugin-transform-react-jsx': 7.22.5_@babel+core@7.22.8 @@ -705,6 +707,17 @@ packages: '@babel/plugin-syntax-dynamic-import': 7.8.3_@babel+core@7.22.8 dev: true + /@babel/plugin-proposal-explicit-resource-management/7.22.6_@babel+core@7.22.8: + resolution: {integrity: sha512-HWbTYUGDXZX9OjVs5RN5DydRZ48PD9QNe10Jgiwz+G0i8ESmQhKMHT01AyivzKzVudxLpXeEHpbhYOBPZLXQ0Q==} + engines: {node: '>=6.9.0'} + peerDependencies: + '@babel/core': ^7.0.0-0 + dependencies: + '@babel/core': 7.22.8 + '@babel/helper-plugin-utils': 7.22.5 + '@babel/plugin-syntax-explicit-resource-management': 7.22.5_@babel+core@7.22.8 + dev: true + /@babel/plugin-proposal-export-namespace-from/7.18.9_@babel+core@7.22.8: resolution: {integrity: sha512-k1NtHyOMvlDDFeb9G5PhUXuGj8m/wiwojgQVEhJ/fsVsMCpLyOP4h0uGEjYJKrRI+EVPlb5Jk+Gt9P97lOGwtA==} engines: {node: '>=6.9.0'} @@ -873,6 +886,16 @@ packages: '@babel/helper-plugin-utils': 7.22.5 dev: true + /@babel/plugin-syntax-explicit-resource-management/7.22.5_@babel+core@7.22.8: + resolution: {integrity: sha512-vokH/rTR4m9hlcxXXL0CPnpoGHUbZ6gfI3kq/UZSwrF9qGo/LxWqEP0qWYqkt5kc6/jrCOTaBeYw+lYleEFtLA==} + engines: {node: '>=6.9.0'} + peerDependencies: + '@babel/core': ^7.0.0-0 + dependencies: + '@babel/core': 7.22.8 + '@babel/helper-plugin-utils': 7.22.5 + dev: true + /@babel/plugin-syntax-export-namespace-from/7.8.3_@babel+core@7.22.8: resolution: {integrity: sha512-MXf5laXo6c1IbEbegDmzGPwGNTsHZmEy6QGznu5Sh2UCWvueywb2ee+CCE4zQiZstxU9BMoQO9i6zUFSY0Kj0Q==} peerDependencies: diff --git a/test/browser/babel.js b/test/browser/babel.js index cfa6e10f1..285a2a49c 100644 --- a/test/browser/babel.js +++ b/test/browser/babel.js @@ -8,6 +8,7 @@ import transformReactJsx from "@babel/plugin-transform-react-jsx"; import { transform } from "@babel/standalone"; // @ts-expect-error import signalsTransform from "@preact/signals-react-transform"; +import explicitResourceManagement from "@babel/plugin-proposal-explicit-resource-management"; globalThis.transformSignalCode = function transformSignalCode(code, options) { const signalsPluginConfig = [signalsTransform]; @@ -21,6 +22,7 @@ globalThis.transformSignalCode = function transformSignalCode(code, options) { syntaxJsx, [transformReactJsx, { runtime: "automatic" }], [transformEsm, { importInterop: "none", loose: true, strict: true }], + explicitResourceManagement, ], });