2
\$\begingroup\$

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
asked Jul 16, 2024 at 8:14
\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

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.

toolic
14.6k5 gold badges29 silver badges204 bronze badges
answered Jul 23, 2024 at 12:29
\$\endgroup\$
1
  • 1
    \$\begingroup\$ I finally decided to separate the delete methods into two delete and clear methods, like the standard methods for the Map() and Set() objects, to make the API more transparent. I also used Symbol() for internal properties to avoid name collisions. \$\endgroup\$ Commented Jul 31, 2024 at 8:20

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.