Remove auto tracking using React internals from signals-react package (…

…#467)

* Add auto package to react package.json

* Remove auto tracking using React internals from signals-react

* Make runtime package external to @preact/signals-react package to avoid duplicate code when building

* Update Readmes

* Change opt-in/opt-out comment to `@useSignals` and `@noUseSignals`

* Add note about not supporting signal DOM attributes

* Add some more limitations and migration notes

* Update vite.config.ts with package aliases and module preload options

.changeset/funny-falcons-pretend.md

@@ -0,0 +1,11 @@
+---
+"@preact/signals-react": major
+---
+
+Remove auto tracking using React internals from signals-react package
+
+Before this change, importing `@preact/signals-react` would invoke side effects that hook into React internals to automatically track signals. This change removes those side effects and requires consumers to update their code to continue using signals in React.
+
+We made this breaking change because the mechanism we were using to automatically track signals was fragile and not reliable. We've had multiple issues reported where signals were not being tracked correctly. It would also lead to unexpected errors that were hard to debug.
+
+For some consumers and apps though, the current mechanism does work. If you'd like to continue using this mechanism, simply add `import "@preact/signals/auto";` to the root of your app where you call `ReactDOM.render`. For our newly supported ways of using signals in React, check out the new Readme for `@preact/signals-react`.

.changeset/warm-apples-applaud.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-react-transform": minor
+---
+
+Change opt-in/opt-out comment to `@useSignals` and `@noUseSignals`. Previous comments (`@trackSignals` & `@noTrackSignals`) still supported but deprecated.

docs/vite.config.ts

@@ -1,13 +1,14 @@
 import { defineConfig, Plugin, Connect } from "vite";
 import preact from "@preact/preset-vite";
-import { resolve, posix } from "path";
+import { resolve, posix, join } from "path";
 import fs from "fs";
 
+const root = resolve(__dirname, "../packages");
+
 // Automatically set up aliases for monorepo packages.
 // Uses built packages in prod, "source" field in dev.
 function packages(prod: boolean) {
 	const alias: Record<string, string> = {};
-	const root = resolve(__dirname, "../packages");
 	for (let name of fs.readdirSync(root)) {
 		if (name[0] === ".") continue;
 		const p = resolve(root, name, "package.json");
@@ -37,7 +38,7 @@ export default defineConfig(env => ({
 		include: ["preact/jsx-runtime", "preact/jsx-dev-runtime"],
 	},
 	build: {
-		polyfillModulePreload: false,
+		modulePreload: { polyfill: false },
 		cssCodeSplit: false,
 		rollupOptions: {
 			output: {
@@ -55,15 +56,30 @@ export default defineConfig(env => ({
 	},
 	resolve: {
 		extensions: [".ts", ".tsx", ".js", ".jsx", ".d.ts"],
-		alias: env.mode === "production" ? {} : packages(false),
+		alias:
+			env.mode === "production"
+				? {
+						// Vite can't resolve main packages referring to their own sub
+						// packages at build time (aka @preact/signals-react ->
+						// @preact/signals-react/runtime) because pnpm symlinks resolve to
+						// the actual paths so when Vite climbs up the directory tree to
+						// find the parent node_modules to then resolve back down, it
+						// doesn't find the parent node_modules since our source is not in
+						// one, as expected. I'm working around this by just mainly aliasing
+						// the package that needs to be resolved.
+						"@preact/signals-react/runtime": join(root, "react/runtime"),
+				  }
+				: packages(false),
 	},
 }));
 
 function unsetPreactAliases(): Plugin {
 	return {
 		name: "remove react aliases",
 		config(config) {
-			const aliases = config.resolve!.alias!;
+			const aliases = config.resolve?.alias;
+			if (aliases == null) return;
+
 			["react", "react-dom", "react-dom/test-utils"].forEach(pkg => {
 				// @ts-ignore-next-line
 				delete aliases[pkg];

package.json

@@ -4,10 +4,10 @@
   "scripts": {
     "prebuild": "rimraf packages/core/dist/ packages/preact/dist",
     "build": "pnpm build:core && pnpm build:preact && pnpm build:react-runtime && pnpm build:react-auto && pnpm build:react && pnpm build:react-transform",
-    "_build": "microbundle --raw --globals @preact/signals-core=preactSignalsCore,preact/hooks=preactHooks",
+    "_build": "microbundle --raw --globals @preact/signals-core=preactSignalsCore,preact/hooks=preactHooks,@preact/signals-react/runtime=reactSignalsRuntime",
     "build:core": "pnpm _build --cwd packages/core && pnpm postbuild:core",
     "build:preact": "pnpm _build --cwd packages/preact && pnpm postbuild:preact",
-    "build:react": "pnpm _build --cwd packages/react && pnpm postbuild:react",
+    "build:react": "pnpm _build --cwd packages/react --external \"react,@preact/signals-react/runtime,@preact/signals-core\" && pnpm postbuild:react",
     "build:react-auto": "pnpm _build --cwd packages/react/auto && pnpm postbuild:react-auto",
     "build:react-runtime": "pnpm _build --cwd packages/react/runtime && pnpm postbuild:react-runtime",
     "build:react-transform": "pnpm _build --no-compress --cwd packages/react-transform",

packages/react-transform/README.md

@@ -12,16 +12,14 @@ Read the [announcement post](https://preactjs.com/blog/introducing-signals/) to
 ## Installation:
 
 ```sh
-npm install @preact/signals-react-transform
+npm i --save-dev @preact/signals-react-transform
 ```
 
 ## Usage
 
-This package works with the `@preact/signals-react` package to integrate signals into React. You use the `@preact/signals-react` package to setup and access signals inside your components and this package is one way to automatically subscribe your components to rerender when the signals you use change.
+This package works with the `@preact/signals-react` package to integrate signals into React. You use the `@preact/signals-react` package to setup and access signals inside your components and this package is one way to automatically subscribe your components to rerender when the signals you use change. To understand how to use signals in your components, check out the [Signals React documentation](../react/README.md).
 
-To understand how to use signals in your components, check out the [Signals React documentation](../react/README.md). This babel transform is one of a couple different ways to use signals in React. To see other ways, including integrations that don't require a build step, see the [Signals React documentation](../react/README.md).
-
-Then, setup the transform plugin in your Babel config:
+To setup the transform plugin, add the following to your Babel config:
 
 ```js
 // babel.config.js
@@ -52,18 +50,18 @@ import { signal, useSignals } from "@preact/signals-react";
 const count = signal(0);
 
 function CounterValue() {
-	const effect = useSignals();
+	const store = useSignals(1);
 	try {
 		// Whenever the `count` signal is updated, we'll
 		// re-render this component automatically for you
 		return <p>Value: {count.value}</p>;
 	} finally {
-		effect.endTracking();
+		store.f();
 	}
 }
 ```
 
-The `useSignals` hook setups the machinery to observe what signals are used inside the component and then automatically re-render the component when those signals change. The `endTracking` function notifies the tracking mechanism that this component has finished rendering. When your component unmounts, it also unsubscribes from all signals it was using.
+The `useSignals` hook setups the machinery to observe what signals are used inside the component and then automatically re-render the component when those signals change. The `f()` function notifies the tracking mechanism that this component has finished rendering. When your component unmounts, it also unsubscribes from all signals it was using.
 
 Fundamentally, this Babel transform needs to answer two questions in order to know whether to transform a function:
 
@@ -72,35 +70,34 @@ Fundamentally, this Babel transform needs to answer two questions in order to kn
 
 Currently we use the following heuristics to answer these questions:
 
-1. A function is a component if it has a capitalized name (e.g. `function MyComponent() {}`), contains JSX, and is declared at module scope.
+1. A function is a component if it has a capitalized name (e.g. `function MyComponent() {}`) and contains JSX.
 2. If a function's body includes a member expression referencing `.value` (i.e. `something.value`), we assume it's a signal.
 
-If your function/component meets these criteria, this plugin will transform it. If not, it will be left alone. If you have a function that uses signals but does not meet these criteria (e.g. a function that manually calls `createElement` instead of using JSX), you can add a comment with the string `@trackSignals` to instruct this plugin to transform this function. You can also manually opt-out of transforming a function by adding a comment with the string `@noTrackSignals`.
+If your function/component meets these criteria, this plugin will transform it. If not, it will be left alone. If you have a function that uses signals but does not meet these criteria (e.g. a function that manually calls `createElement` instead of using JSX), you can add a comment with the string `@useSignals` to instruct this plugin to transform this function. You can also manually opt-out of transforming a function by adding a comment with the string `@noUseSignals`.
 
 ```js
 // This function will be transformed
-/** @trackSignals */
+/** @useSignals */
 function MyComponent() {
 	return createElement("h1", null, signal.value);
 }
 
 // This function will not be transformed
-/** @noTrackSignals */
+/** @noUseSignals */
 function MyComponent() {
 	return <p>{signal.value}</p>;
 }
 ```
 
-Note, this plugin will not transform higher-order components (HOCs) that wrap other components. If you have an HOC that uses signals, you can use the `@trackSignals` comment to transform the body of the higher-order component.
-
 ## Plugin Options
 
 ### `mode`
 
-The `mode` option enables you to control how the plugin transforms your code. There are two modes:
+The `mode` option enables you to control how the plugin transforms your code. There are three modes:
 
 - `mode: "auto"` (default): This mode will automatically transform any function that meets the criteria described above. This is the easiest way to get started with signals.
-- `mode: "manual"`: This mode will only transform functions that have a comment with the string `@trackSignals`. This is useful if you want to manually control which functions are transformed.
+- `mode: "manual"`: This mode will only transform functions that have a comment with the string `@useSignals`. This is useful if you want to manually control which functions are transformed.
+- `mode: "all"`: This mode will transform all functions that appear to be Components, regardless of whether or not they use signals. This is useful if you are starting a new project and want to use signals everywhere.
 
 ```js
 // babel.config.js
@@ -116,6 +113,28 @@ module.exports = {
 };
 ```
 
+### `importSource`
+
+The `importSource` option enables you to control where the `useSignals` hook is imported from. By default, it will import from `@preact/signals-react`. This is useful if you want to wrap the exports of the `@preact/signals-react` package to provide customized behavior or if you want to use a different package entirely. Note: if you use a different package, you'll need to make sure that it exports a `useSignals` hook with the same API & behavior as the one in `@preact/signals-react`.
+
+```js
+// babel.config.js
+module.exports = {
+	plugins: [
+		[
+			"@preact/signals-react-transform",
+			{
+				importSource: "my-signals-package",
+			},
+		],
+	],
+};
+```
+
+## Logging
+
+This plugin uses the [`debug`](https://www.npmjs.com/package/debug) package to log information about what it's doing. To enable logging, set the `DEBUG` environment variable to `signals:react-transform:*`.
+
 ## License
 
 `MIT`, see the [LICENSE](../../LICENSE) file.

packages/react-transform/src/index.ts

@@ -15,8 +15,8 @@ interface PluginArgs {
 	template: typeof BabelTemplate;
 }
 
-const optOutCommentIdentifier = /(^|\s)@noTrackSignals(\s|$)/;
-const optInCommentIdentifier = /(^|\s)@trackSignals(\s|$)/;
+const optOutCommentIdentifier = /(^|\s)@no(Use|Track)Signals(\s|$)/;
+const optInCommentIdentifier = /(^|\s)@(use|track)Signals(\s|$)/;
 const dataNamespace = "@preact/signals-react-transform";
 const defaultImportSource = "@preact/signals-react/runtime";
 const importName = "useSignals";
@@ -428,7 +428,7 @@ export interface PluginOptions {
 	/**
 	 * Specify the mode to use:
 	 * - `auto`: Automatically wrap all components that use signals.
-	 * - `manual`: Only wrap components that are annotated with `@trackSignals` in a JSX comment.
+	 * - `manual`: Only wrap components that are annotated with `@useSignals` in a JSX comment.
 	 * - `all`: Makes all components reactive to signals.
 	 */
 	mode?: "auto" | "manual" | "all";

packages/react-transform/test/browser/e2e.test.tsx

@@ -399,7 +399,7 @@ describe("React Signals babel transfrom - browser E2E tests", () => {
 			export const name = signal("John");
 
 			// Ambiguous if this function is gonna be a hook or component
-			/** @trackSignals */
+			/** @useSignals */
 			function usename() {
 				return name.value;
 			}

packages/react-transform/test/node/helpers.ts

@@ -218,8 +218,8 @@ function generateParams(count?: ParamsConfig): string {
 }
 
 function generateComment(comment?: CommentKind): string {
-	if (comment === "opt-out") return "/* @noTrackSignals */\n";
-	if (comment === "opt-in") return "/* @trackSignals */\n";
+	if (comment === "opt-out") return "/* @noUseSignals */\n";
+	if (comment === "opt-in") return "/* @useSignals */\n";
 	return "";
 }
 

packages/react-transform/test/node/index.test.tsx

@@ -246,8 +246,8 @@ describe("React Signals Babel Transform", () => {
 		it("opt-out comment overrides opt-in comment", () => {
 			const inputCode = `
 				/**
-				 * @noTrackSignals
-				 * @trackSignals
+				 * @noUseSignals
+				 * @useSignals
 				 */
 				function MyComponent() {
 					return <div>{signal.value}</div>;
@@ -302,8 +302,8 @@ describe("React Signals Babel Transform", () => {
 		it("opt-out comment overrides opt-in comment", () => {
 			const inputCode = `
 				/**
-				 * @noTrackSignals
-				 * @trackSignals
+				 * @noUseSignals
+				 * @useSignals
 				 */
 				function MyComponent() {
 					return <div>{signal.value}</div>;
@@ -330,7 +330,7 @@ describe("React Signals Babel Transform", () => {
 	describe("all mode transformations", () => {
 		it("skips transforming arrow function component with leading opt-out JSDoc comment before variable declaration", () => {
 			const inputCode = `
-				/** @noTrackSignals */
+				/** @noUseSignals */
 				const MyComponent = () => {
 					return <div>{signal.value}</div>;
 				};
@@ -343,7 +343,7 @@ describe("React Signals Babel Transform", () => {
 
 		it("skips transforming function declaration components with leading opt-out JSDoc comment", () => {
 			const inputCode = `
-				/** @noTrackSignals */
+				/** @noUseSignals */
 				function MyComponent() {
 					return <div>{signal.value}</div>;
 				}

packages/react/README.md

@@ -26,15 +26,25 @@ npm install @preact/signals-react
 
 ## React Integration
 
-> Note: The React integration plugs into some React internals and may break unexpectedly in future versions of React. If you are using Signals with React and encounter errors such as "Rendered more hooks than during previous render", "Should have a queue. This is likely a bug in React." or "Cannot redefine property: createElement" please open an issue here.
-
 The React integration can be installed via:
 
 ```sh
 npm install @preact/signals-react
 ```
 
-Similar to the Preact integration, the React adapter allows you to access signals directly inside your components and will automatically subscribe to them.
+We have a couple of options for integrating Signals into React. The recommended approach is to use the Babel transform to automatically make your components that use signals reactive.
+
+### Babel Transform
+
+Install the Babel transform package (`npm i --save-dev @preact/signals-react-transform`) and add the following to your Babel config:
+
+```json
+{
+	"plugins": [["module:@preact/signals-react-transform"]]
+}
+```
+
+This will automatically transform your components to be reactive. You can then use signals directly inside your components.
 
 ```js
 import { signal } from "@preact/signals-react";
@@ -48,6 +58,23 @@ function CounterValue() {
 }
 ```
 
+See the [Readme for the Babel plugin](../babel-plugin-signals-react/README.md) for more details about how the transform works and configuring it.
+
+### `useSignals` hook
+
+If you can't use the Babel transform, you can directly call the `useSignals` hook to make your components reactive.
+
+```js
+import { useSignals } from "@preact/signals-react";
+
+const count = signal(0);
+
+function CounterValue() {
+	useSignals();
+	return <p>Value: {count.value}</p>;
+}
+```
+
 ### Hooks
 
 If you need to instantiate new signals inside your components, you can use the `useSignal` or `useComputed` hook.
@@ -97,6 +124,42 @@ To opt into this optimization, simply pass the signal directly instead of access
 > **Note**
 > The content is wrapped in a React Fragment due to React 18's newer, more strict children types.
 
+## Limitations
+
+This version of React integration does not support passing signals as DOM attributes. Support for this may be added at a later date.
+
+Using signals into render props is not recommended. In this situation, the component that reads the signal is the component that calls the render prop, which may or may not be hooked up to track signals. For example:
+
+```js
+const count = signal(0);
+
+function ShowCount({ getCount }) {
+	return <div>{getCount()}</div>;
+}
+
+function App() {
+	return <ShowCount getCount={() => count.value} />;
+}
+```
+
+Here, the `ShowCount` component is the one that accesses `count.value` at runtime since it invokes `getCount`, so it needs to be hooked up to track signals. However, since it doesn't statically access the signal, the Babel transform won't transform it by default. One fix is to set `mode: all` in the Babel plugin's config, which will transform all components. Another workaround is put the return of the render prop into it's own component and then return that from your render prop. In the following example, the `Count` component statically accesses the signal, so it will be transformed by default.
+
+```js
+const count = signal(0);
+
+function ShowCount({ getCount }) {
+	return <div>{getCount()}</div>;
+}
+
+const Count = () => <>{count.value}</>;
+
+function App() {
+	return <ShowCount getCount={() => <Count />} />;
+}
+```
+
+Similar issues exist with using object getters & setters. Since the it isn't easily statically analyzable that a getter or setter is backed by a signal, the Babel plugin may miss some components that use signals in this way. Similarly, setting Babel's plugin to `mode: all` will fix this issue.
+
 ## License
 
 `MIT`, see the [LICENSE](../../LICENSE) file.