fix: Ensure that text effects get disposed

packages/preact/src/index.ts

@@ -33,6 +33,17 @@ const HAS_PENDING_UPDATE = 1 << 0;
 const HAS_HOOK_STATE = 1 << 1;
 const HAS_COMPUTEDS = 1 << 2;
 
+let oldNotify: (this: Effect) => void,
+	effectsQueue: Array<Effect> = [],
+	domQueue: Array<Effect> = [];
+
+// Capture the original `Effect.prototype._notify` method so that we can install
+// custom `._notify`s for each different use-case but still call the original
+// implementation in the end. Dispose the temporary effect immediately afterwards.
+effect(function (this: Effect) {
+	oldNotify = this._notify;
+})();
+
 // Install a Preact options hook
 function hook<T extends OptionsTypes>(hookName: T, hookFn: HookFn<T>) {
 	// @ts-ignore-next-line private options hooks usage
@@ -79,7 +90,7 @@ function SignalValue(this: AugmentedComponent, { data }: { data: Signal }) {
 	const currentSignal = useSignal(data);
 	currentSignal.value = data;
 
-	const s = useMemo(() => {
+	const [isText, s] = useMemo(() => {
 		let self = this;
 		// mark the parent component as having computeds so it gets optimized
 		let v = this.__v;
@@ -90,38 +101,51 @@ function SignalValue(this: AugmentedComponent, { data }: { data: Signal }) {
 			}
 		}
 
-		const wrappedSignal = computed(function (this: Effect) {
-			let data = currentSignal.value;
-			let s = data.value;
+		const wrappedSignal = computed(() => {
+			let s = currentSignal.value.value;
 			return s === 0 ? 0 : s === true ? "" : s || "";
 		});
 
-		const isText = computed(
-			() => isValidElement(wrappedSignal.value) || this.base?.nodeType !== 3
-		);
-
-		this._updater!._callback = () => {
-			if (isValidElement(s.peek()) || this.base?.nodeType !== 3) {
-				this._updateFlags |= HAS_PENDING_UPDATE;
-				this.setState({});
-				return;
-			}
-			(this.base as Text).data = s.peek();
-		};
+		const isText = computed(() => !isValidElement(wrappedSignal.value));
 
-		effect(function (this: Effect) {
-			if (!oldNotify) oldNotify = this._notify;
+		// Update text nodes directly without rerendering when the new value
+		// is also text.
+		const dispose = effect(function (this: Effect) {
 			this._notify = notifyDomUpdates;
-			const val = wrappedSignal.value;
-			if (isText.value && self.base) {
-				(self.base as Text).data = val;
+
+			// Subscribe to wrappedSignal updates only when its values are text...
+			if (isText.value) {
+				// ...but regardless of `self.base`'s current value, as it can be
+				// undefined before mounting or a non-text node. In both of those cases
+				// the update gets handled by a full rerender.
+				const value = wrappedSignal.value;
+				if (self.base?.nodeType === 3) {
+					(self.base as Text).data = value;
+				}
 			}
 		});
 
-		return wrappedSignal;
+		// Piggyback this._updater's disposal to ensure that the text updater effect
+		// above also gets disposed on unmount.
+		const oldDispose = this._updater!._dispose;
+		this._updater!._dispose = function () {
+			dispose();
+			oldDispose.call(this);
+		};
+
+		return [isText, wrappedSignal];
 	}, []);
 
-	return s.value;
+	// Rerender the component whenever `data.value` changes from a VNode
+	// to another VNode, from text to a VNode, or from a VNode to text.
+	// That is, everything else except text-to-text updates.
+	//
+	// This also ensures that the backing DOM node types gets updated to
+	// text nodes and back when needed.
+	//
+	// For text-to-text updates, `.peek()` is used to skip full rerenders,
+	// leaving them to the optimized path above.
+	return isText.value ? s.peek() : s.value;
 }
 SignalValue.displayName = "_st";
 
@@ -254,7 +278,6 @@ function createPropUpdater(
 			props = newProps;
 		},
 		_dispose: effect(function (this: Effect) {
-			if (!oldNotify) oldNotify = this._notify;
 			this._notify = notifyDomUpdates;
 			const value = changeSignal.value.value;
 			// If Preact just rendered this value, don't render it again:
@@ -365,10 +388,6 @@ export function useComputed<T>(compute: () => T) {
 	return useMemo(() => computed<T>(() => $compute.current()), []);
 }
 
-let oldNotify: (this: Effect) => void,
-	effectsQueue: Array<Effect> = [],
-	domQueue: Array<Effect> = [];
-
 const deferEffects =
 	typeof requestAnimationFrame === "undefined"
 		? setTimeout
@@ -416,7 +435,6 @@ export function useSignalEffect(cb: () => void | (() => void)) {
 
 	useEffect(() => {
 		return effect(function (this: Effect) {
-			if (!oldNotify) oldNotify = this._notify;
 			this._notify = notifyEffects;
 			return callback.current();
 		});