Merge branch 'main' into hookless

.changeset/funny-geese-hear.md

@@ -0,0 +1,6 @@
+---
+"@preact/signals": patch
+"@preact/signals-react": patch
+---
+
+Fix hook names being mangled

.changeset/pretty-students-look.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-core": patch
+---
+
+Simplify effect change checking (and make effect cycle detection more accurate as a side-effect)

.changeset/shy-cobras-try.md

@@ -0,0 +1,5 @@
+---
+"@preact/signals-core": patch
+---
+
+Simplify Node book keeping code

karma.conf.js

@@ -25,6 +25,7 @@ var localLaunchers = {
 			"--no-gpu",
 			// Without a remote debugging port, Google Chrome exits immediately.
 			"--remote-debugging-port=9333",
+			"--js-flags=--expose-gc",
 		],
 	},
 };
@@ -157,7 +158,7 @@ function createEsbuildPlugin() {
 							coverage && [
 								"istanbul",
 								{
-									include: minify ? "**/dist/**/*.js" : "**/src/**/*.js",
+									include: minify ? "**/dist/**/*.js" : "**/src/**/*.{ts,js}",
 								},
 							],
 							minify && renamePlugin,

mangle.json

@@ -5,6 +5,11 @@
   },
   "minify": {
     "mangle": {
+      "reserved": [
+        "useSignal",
+        "useComputed",
+        "useSignalEffect"
+      ],
       "keep_classnames": true,
       "properties": {
         "regex": "^_[^_]",

packages/core/CHANGELOG.md

@@ -1,5 +1,15 @@
 # @preact/signals-core
 
+## 1.2.1
+
+### Patch Changes
+
+- [#205](https://github.com/preactjs/signals/pull/205) [`4b73164`](https://github.com/preactjs/signals/commit/4b7316497aee03413f91e9f714cdcf9f553e39d9) Thanks [@jviide](https://github.com/jviide)! - Use the same tracking logic for both effects and computeds. This ensures that effects are only called whenever any of their dependencies changes. If they all stay the same, then the effect will not be invoked.
+
+* [#207](https://github.com/preactjs/signals/pull/207) [`57fd2e7`](https://github.com/preactjs/signals/commit/57fd2e723528a36cc5d4ebf09ba34178aa84c879) Thanks [@jviide](https://github.com/jviide)! - Fix effect disposal when cleanup throws
+
+- [#209](https://github.com/preactjs/signals/pull/209) [`49756ae`](https://github.com/preactjs/signals/commit/49756aef28fe12c6ae6b801224bf5ae608ddf562) Thanks [@jviide](https://github.com/jviide)! - Optimize dependency value change checks by allowing earlier exists from the loop
+
 ## 1.2.0
 
 ### Minor Changes

packages/core/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@preact/signals-core",
-	"version": "1.2.0",
+	"version": "1.2.1",
 	"license": "MIT",
 	"description": "",
 	"keywords": [],

packages/core/src/index.ts

@@ -10,18 +10,9 @@ const DISPOSED = 1 << 3;
 const HAS_ERROR = 1 << 4;
 const TRACKING = 1 << 5;
 
-// Flags for Nodes.
-const NODE_FREE = 1 << 0;
-const NODE_SUBSCRIBED = 1 << 1;
-
 // A linked list node used to track dependencies (sources) and dependents (targets).
 // Also used to remember the source's last version number that the target saw.
 type Node = {
-	// A node may have the following flags:
-	//  NODE_FREE when it's unclear whether the source is still a dependency of the target
-	//  NODE_SUBSCRIBED when the target has subscribed to listen change notifications from the source
-	_flags: number;
-
 	// A source whose value the target depends on.
 	_source: Signal;
 	_prevSource?: Node;
@@ -35,6 +26,7 @@ type Node = {
 	// The version number of the source that target has last seen. We use version numbers
 	// instead of storing the source value, because source values can take arbitrary amount
 	// of memory, and computeds could hang on to them forever because they're lazily evaluated.
+	// Use the special value -1 to mark potentially unused but recyclable nodes.
 	_version: number;
 
 	// Used to remember & roll back the source's previous `._node` value when entering &
@@ -66,7 +58,7 @@ function endBatch() {
 			effect._nextBatchedEffect = undefined;
 			effect._flags &= ~NOTIFIED;
 
-			if (!(effect._flags & DISPOSED) && effect._flags & OUTDATED) {
+			if (!(effect._flags & DISPOSED) && needsToRecompute(effect)) {
 				try {
 					effect._callback();
 				} catch (err) {
@@ -121,7 +113,6 @@ function addDependency(signal: Signal): Node | undefined {
 		// `signal` is a new dependency. Create a new node dependency node, move it
 		//  to the front of the current context's dependency list.
 		node = {
-			_flags: 0,
 			_version: 0,
 			_source: signal,
 			_prevSource: undefined,
@@ -140,26 +131,22 @@ function addDependency(signal: Signal): Node | undefined {
 			signal._subscribe(node);
 		}
 		return node;
-	} else if (node._flags & NODE_FREE) {
-		// `signal` is an existing dependency from a previous evaluation. Reuse the dependency
-		// node and move it to the front of the evaluation context's dependency list.
-		node._flags &= ~NODE_FREE;
-
-		const head = evalContext._sources;
-		if (node !== head) {
-			const prev = node._prevSource;
-			const next = node._nextSource;
-			if (prev !== undefined) {
-				prev._nextSource = next;
-			}
-			if (next !== undefined) {
-				next._prevSource = prev;
-			}
-			if (head !== undefined) {
-				head._prevSource = node;
+	} else if (node._version === -1) {
+		// `signal` is an existing dependency from a previous evaluation. Reuse it.
+		node._version = 0;
+
+		// If `node` is not already the current head of the dependency list (i.e.
+		// there is a previous node in the list), then make `node` the new head.
+		if (node._prevSource !== undefined) {
+			node._prevSource._nextSource = node._nextSource;
+			if (node._nextSource !== undefined) {
+				node._nextSource._prevSource = node._prevSource;
 			}
 			node._prevSource = undefined;
-			node._nextSource = head;
+			node._nextSource = evalContext._sources;
+			// evalCotext._sources must be !== undefined (and !== node), because
+			// `node` was originally pointing to some previous node.
+			evalContext._sources!._prevSource = node;
 			evalContext._sources = node;
 		}
 
@@ -174,7 +161,10 @@ declare class Signal<T = any> {
 	/** @internal */
 	_value: unknown;
 
-	/** @internal */
+	/** @internal
+	 * Version numbers should always be >= 0, because the special value -1 is used
+	 * by Nodes to signify potentially unused but recyclable notes.
+	 */
 	_version: number;
 
 	/** @internal */
@@ -219,10 +209,8 @@ Signal.prototype._refresh = function () {
 };
 
 Signal.prototype._subscribe = function (node) {
-	if (!(node._flags & NODE_SUBSCRIBED)) {
-		node._flags |= NODE_SUBSCRIBED;
+	if (this._targets !== node && node._prevTarget === undefined) {
 		node._nextTarget = this._targets;
-
 		if (this._targets !== undefined) {
 			this._targets._prevTarget = node;
 		}
@@ -231,22 +219,18 @@ Signal.prototype._subscribe = function (node) {
 };
 
 Signal.prototype._unsubscribe = function (node) {
-	if (node._flags & NODE_SUBSCRIBED) {
-		node._flags &= ~NODE_SUBSCRIBED;
-
-		const prev = node._prevTarget;
-		const next = node._nextTarget;
-		if (prev !== undefined) {
-			prev._nextTarget = next;
-			node._prevTarget = undefined;
-		}
-		if (next !== undefined) {
-			next._prevTarget = prev;
-			node._nextTarget = undefined;
-		}
-		if (node === this._targets) {
-			this._targets = next;
-		}
+	const prev = node._prevTarget;
+	const next = node._nextTarget;
+	if (prev !== undefined) {
+		prev._nextTarget = next;
+		node._prevTarget = undefined;
+	}
+	if (next !== undefined) {
+		next._prevTarget = prev;
+		node._nextTarget = undefined;
+	}
+	if (node === this._targets) {
+		this._targets = next;
 	}
 };
 
@@ -314,6 +298,31 @@ function signal<T>(value: T): Signal<T> {
 	return new Signal(value);
 }
 
+function needsToRecompute(target: Computed | Effect): boolean {
+	// Check the dependencies for changed values. The dependency list is already
+	// in order of use. Therefore if multiple dependencies have changed values, only
+	// the first used dependency is re-evaluated at this point.
+	for (
+		let node = target._sources;
+		node !== undefined;
+		node = node._nextSource
+	) {
+		// If there's a new version of the dependency before or after refreshing,
+		// or the dependency has something blocking it from refreshing at all (e.g. a
+		// dependency cycle), then we need to recompute.
+		if (
+			node._source._version !== node._version ||
+			!node._source._refresh() ||
+			node._source._version !== node._version
+		) {
+			return true;
+		}
+	}
+	// If none of the dependencies have changed values since last recompute then the
+	// there's no need to recompute.
+	return false;
+}
+
 function prepareSources(target: Computed | Effect) {
 	for (
 		let node = target._sources;
@@ -325,7 +334,7 @@ function prepareSources(target: Computed | Effect) {
 			node._rollbackNode = rollbackNode;
 		}
 		node._source._node = node;
-		node._flags |= NODE_FREE;
+		node._version = -1;
 	}
 }
 
@@ -340,7 +349,7 @@ function cleanupSources(target: Computed | Effect) {
 	let sources = undefined;
 	while (node !== undefined) {
 		const next = node._nextSource;
-		if (node._flags & NODE_FREE) {
+		if (node._version === -1) {
 			node._source._unsubscribe(node);
 			node._nextSource = undefined;
 		} else {
@@ -407,25 +416,9 @@ Computed.prototype._refresh = function () {
 	// Mark this computed signal running before checking the dependencies for value
 	// changes, so that the RUNNIN flag can be used to notice cyclical dependencies.
 	this._flags |= RUNNING;
-	if (this._version > 0) {
-		// Check the dependencies for changed values. The dependency list is already
-		// in order of use. Therefore if multiple dependencies have changed values, only
-		// the first used dependency is re-evaluated at this point.
-		let node = this._sources;
-		while (node !== undefined) {
-			// If a dependency has something blocking it from refreshing (e.g. a dependency
-			// cycle) or there's a new version of the dependency, then we need to recompute.
-			if (!node._source._refresh() || node._source._version !== node._version) {
-				break;
-			}
-			node = node._nextSource;
-		}
-		// If none of the dependencies have changed values since last recompute then the
-		// computed value can't have changed.
-		if (node === undefined) {
-			this._flags &= ~RUNNING;
-			return true;
-		}
+	if (this._version > 0 && !needsToRecompute(this)) {
+		this._flags &= ~RUNNING;
+		return true;
 	}
 
 	const prevContext = evalContext;
@@ -550,6 +543,8 @@ function cleanupEffect(effect: Effect) {
 			cleanup();
 		} catch (err) {
 			effect._flags &= ~RUNNING;
+			effect._flags |= DISPOSED;
+			disposeEffect(effect);
 			throw err;
 		} finally {
 			evalContext = prevContext;
@@ -566,6 +561,7 @@ function disposeEffect(effect: Effect) {
 	) {
 		node._source._unsubscribe(node);
 	}
+	effect._compute = undefined;
 	effect._sources = undefined;
 
 	cleanupEffect(effect);
@@ -586,7 +582,7 @@ function endEffect(this: Effect, prevContext?: Computed | Effect) {
 }
 
 declare class Effect {
-	_compute: () => unknown;
+	_compute?: () => unknown;
 	_cleanup?: unknown;
 	_sources?: Node;
 	_nextBatchedEffect?: Effect;
@@ -605,13 +601,13 @@ function Effect(this: Effect, compute: () => void) {
 	this._cleanup = undefined;
 	this._sources = undefined;
 	this._nextBatchedEffect = undefined;
-	this._flags = OUTDATED | TRACKING;
+	this._flags = TRACKING;
 }
 
 Effect.prototype._callback = function () {
 	const finish = this._start();
 	try {
-		if (!(this._flags & DISPOSED)) {
+		if (!(this._flags & DISPOSED) && this._compute !== undefined) {
 			this._cleanup = this._compute();
 		}
 	} finally {
@@ -625,19 +621,18 @@ Effect.prototype._start = function () {
 	}
 	this._flags |= RUNNING;
 	this._flags &= ~DISPOSED;
-	prepareSources(this);
 	cleanupEffect(this);
+	prepareSources(this);
 
 	/*@__INLINE__**/ startBatch();
-	this._flags &= ~OUTDATED;
 	const prevContext = evalContext;
 	evalContext = this;
 	return endEffect.bind(this, prevContext);
 };
 
 Effect.prototype._notify = function () {
 	if (!(this._flags & NOTIFIED)) {
-		this._flags |= NOTIFIED | OUTDATED;
+		this._flags |= NOTIFIED;
 		this._nextBatchedEffect = batchedEffect;
 		batchedEffect = this;
 	}