I've created my own implementation of signals in typescript, inspired by the proposed tc39 specification (https://github.com/tc39/proposal-signals) in the BOM. I think it would be great if I could use it in production, but i don't have anyone around me who can do a code review, so need an outside view.
I'd like to know if my code contains any bad practices (e.g. in terms of types), or if certain parts can let memory accumulate (bad reference deletion), or any other glaring problems.
The source code consists of two parts, the first is about Signals and the second about Reactive functions :
import { Reactive } from "./reactive";
export class Signal<Type> {
value: Type;
/** @internal */
dependencies: Set<Reactive>;
/**
* create a signal
* @param {Type?} value the inital signal state
*/
constructor(value?: Type) {
if (typeof value !== "undefined") {
this.value = value;
}
this.dependencies = new Set();
}
/**
* method used to retrieve the value of a signal
* while adding the current reactive function to the dependencies
* @returns {Type} the signal value
*/
get(): Type {
// retrieve the current reactive function
const reactive = Reactive.current;
if (reactive) {
// add the reative function to dependencies
this.dependencies.add(reactive);
// add the signal to reactive function's dependencies
reactive.dependencies.add(this);
}
return this.value;
}
/**
* method used to update the value of a signal
* while triggering all the reactive functions in the dependencies
* @param {Type} value the new signal value
*/
set(value: Type): Type {
// if the value has changed
if (this.value !== value) {
// update the value
this.value = value;
// trigger all the reactive functions in the dependencies
for (const reactive of this.dependencies) {
reactive.value();
}
}
// return the signal value
return this.value;
}
/**
* method used to remove a reactive function from dependencies or ...
* method used to remove all reactive functions from dependencies
* @param {Reactive?} reactive the reactive function to be cleared
*/
delete(reactive?: Reactive): void {
if (reactive) {
// remove the reactive function from dependencies
this.dependencies.delete(reactive);
// remove the signal from reactive function's dependencies
reactive.dependencies.delete(this);
} else {
// remove the signal from all reactive function's dependencies
for (const reactive of this.dependencies) {
reactive.dependencies.delete(this);
}
// clear all dependencies
this.dependencies.clear();
}
}
}
export class ComputedSignal<Type> extends Signal<Type> {
/** @internal */
computation: (value: any) => Type;
/** @internal */
reactive: Reactive | undefined;
/** @internal */
entry: any;
/**
* @see Signal.constructor create a computed signal
* @param {Function} computation the signal computation function
*/
constructor(computation: (value: any) => Type) {
super();
this.computation = computation;
this.reactive = new Reactive(() => this.set(this.entry));
this.reactive.use();
}
/**
* @see Signal.set executes the calculation function before returning the new value
*/
set(value?: any): Type {
// the current input is saved for signal recalculations
this.entry = value;
// get the computed result
value = this.computation(value);
// return Signal.set
return super.set(value);
}
/**
* @see Signal.delete remove the entry property before clearing dependencies
*/
delete(reactive?: Reactive): void {
// clear entry value
delete this.entry;
if (!reactive && this.reactive) {
// clear reactive computation dependencies
this.reactive.delete();
delete this.reactive;
}
// clear dependencies
super.delete(reactive);
}
}
import { Signal } from "./signal";
export class Reactive {
value: Function;
/** @internal */
dependencies: Set<Signal<any>>;
/** @internal */
registered: boolean;
/** @internal */
static current: Reactive | null = null;
/** @internal */
static initial: Reactive | null = null;
/**
* shortcut for constructor and instance.use
* used to avoid static analysis warnings
* @param func the reactive callback
* @param {...any?} args the reactive function arguments
* @returns {any} the reative function result
*/
static use = (func: Function, ...args: any): any =>
new Reactive(func).use(...args);
/**
* create a reactive function
* @param {Function} func the reactive callback
*/
constructor(func: Function) {
this.value = func;
this.dependencies = new Set();
this.registered = false;
}
/**
* method used to manually add the reactive function to signal dependencies
* @param {Signal} signal the signal in which to add the dependency
*/
add(signal: Signal<any>): void {
this.dependencies.add(signal);
signal.dependencies.add(this);
}
/**
* method used to trigger the reactive function
* while changing the value of the current reactive function
* @param {...any?} args the reactive function arguments
* @returns {any} the reative function result
*/
use(...args: any): any {
// the value of the current reactive function is changed only
// when the function is triggered for the first time
// so the function can be re-triggered manually without any side effects
if (!this.registered) {
this.registered = true;
// switch the current reactive function
Reactive.initial = Reactive.current;
Reactive.current = this;
const value = this.value(...args);
// switch back the current reactive function to initial state
Reactive.current = Reactive.initial;
return value;
} else {
return this.value(...args);
}
}
/**
* method used to remove a signal from dependencies or ...
* method used to remove all signals from dependencies
* @param {Signal?} signal the signal to be cleared
*/
delete(signal?: Signal<any>): void {
if (signal) {
// remove the signal from dependencies
this.dependencies.delete(signal);
// remove the reactive function from signal's dependencies
signal.dependencies.delete(this);
} else {
// remove the reactive function from all signals dependencies
for (const signal of this.dependencies) {
signal.dependencies.delete(this);
}
// clear all dependencies
this.dependencies.clear();
}
}
}
I've written a WIKI section with docs and the usage process (via CDN or NPM) to perform tests: https://github.com/enzoaicardi/reactivity/tree/main/wiki
Small example :
import {
Signal, // used to create a signal
Reactive, // used to create a reactive function
} from "https://cdn.jsdelivr.net/npm/@enzoaicardi/reactivity@latest/esm/reactivity.js";
// setup a signal (named counter) and a reactive function (named counterLog)
const counter = new Signal(0);
const counterLog = new Reactive(() => console.log(counter.get()));
counterLog.use(); // this will trigger counterLog (+ bind dependencies) and print "0" in the console
counter.set(1); // this will update counter value, trigger counterLog and print "1" in the console
1 Answer 1
The infinite loop
I finally found a critical bug in my signal implementation. With the following code I get a "maximum call stack size exceeded" error:
const count = new Signal(1);
const countLog = new Reactive(() => console.log(count.get()));
countLog.use();
// should log : 1
const countUpdate = new Reactive(() => count.set(count.value + 1));
countUpdate.use();
// should log : 2 -> but got Infinite Loop instead
Why do I get an infinite loop? In fact, you'll notice that in the reactive function countUpdate
I never use count.get()
, so it should never have any dependencies.
const countUpdate = new Reactive(() => count.set(count.value + 1));
// dependencies : Set([count]) why ?
However, in reality, when countUpdate()
is called, it will perform a count.set(...)
, which will cause count
to be updated and the countLog
reactive function to be re-executed.
at which point countLog
will execute a count.get()
, but as countLog
is already defined, the value of Reactive.current
is equivalent to countUpdate
. This means that count
is added as a dependency of countUpdate
.
Since countUpdate
updates the value of count
and also has it as a dependency, this results in an infinite loop.
Workaround
To get around this problem, we need to prevent the creation of deep dependencies. This is already the case when building dependencies for reactive functions, thanks to the static property Reactive.current
.
const r1 = new Reactive(() => console.log(signal.get()));
const r2 = new Reactive(() => {
console.log("you are in reactive n°2");
// use reactive n°1 nested
r1.use();
});
// use reactive n°2
r2.use();
// r1 dependencies : Set([signal])
// r2 dependencies : Set([])
However, we don't have this assignment mechanism when the reactive function is subsequently used.
use(...args: any): any {
if (!this.registered) {
this.registered = true;
// ...
} else {
// we don't reassign Reactive.current here (BAD)
return this.value(...args);
}
}
So we need to add this mechanism by refactoring the use()
method:
use(...args: any): any {
// save the initial reactive function state
Reactive.initial = Reactive.current;
// switch the current reactive function, set to `null` if the function is already registered
Reactive.current = this.registered ? null : this;
// toggle registred status
if (!this.registered) {
this.registered = true;
}
// trigger the reactive function and store the value
const value = this.value(...args);
// switch back the current reactive function to initial state
Reactive.current = Reactive.initial;
return value;
}
We then need to make sure that we use the use()
method in the signals when updating:
set(value: Type): Type {
// if the value has changed
if (this.value !== value) {
// update the value
this.value = value;
// trigger all the reactive functions in the dependencies
for (const reactive of this.dependencies) {
// before it was reactive.value()
reactive.use();
}
}
// return the signal value
return this.value;
}
Now the signals work correctly even with nested updates.
-
1\$\begingroup\$ I finally decided to separate the
delete
methods into twodelete
andclear
methods, like the standard methods for theMap()
andSet()
objects, to make the API more transparent. I also usedSymbol()
for internal properties to avoid name collisions. \$\endgroup\$enzoaicardi– enzoaicardi2024年07月31日 08:20:50 +00:00Commented Jul 31, 2024 at 8:20
Explore related questions
See similar questions with these tags.