Proposal to rewrite useDataHook without adding/removing each time listeners

[JeanLucPons]

Hello,

I made a proposal for a new useDataHook() implementation in order to avoid to remove and readd listeners at each rendering and remove them only on willUnmout. The trick is to use directly the function returned by useState() in the listener list. React guaranty a stable handle of this function during component lifecycle. I removed the asynchronous mechanism and "loadable field" management there for clarity.

import { useEffect, useRef, useState } from "react";
import { IDataListener } from "./Field";

/**
 * Retrieves a hook that can be used to listen to data from data sources,
 * such that the component rerenders upon data changes.
 * It also returns a function to determine whether the data is still loading, or has errored.
 * @param options  Configuration options
 * @returns The data hook followed by contextual data
 */
export function useDataHook() : IDataListener {

    // Create the dispatcher, React guaranty that this function is 
    // stable during the component lifecycle
    const [, _dispatch] = useState({});

    // A list of functions to call to remove the passed listener as a dependency
    const dependencyRemovers = useRef([] as (() => void)[]);

    const removeDependencies = () => {
        dependencyRemovers.current.forEach(remove => remove());
        dependencyRemovers.current = [];
    };

    useEffect(() => {
        //Component DidMount
        return () => {
            //Component WillUnmont
            console.log("Component unmounted")
            removeDependencies();
        };
    }, []);

    return(
        // Return the listener which will force an update, and registers whether any data is refreshing
        {
            // Data listener fields
            dispatch: _dispatch,
            registerRemover(remover: () => void) {
                dependencyRemovers.current.push(remover);
            }
        }
    )
};

    protected listeners: Array<React.Dispatch<React.SetStateAction<{}>>> = []

    /**
    * Adds a listener for this field
    * @param listener The listener to add
    */
    protected addListener(listener?: any): void {
        if (isDataListener(listener) && !this.listeners.includes(listener.dispatch)) {
            console.log("addListerner "+util.inspect(listener));
            this.listeners.push(listener.dispatch);
            listener.registerRemover(() => {
                console.log("removeListerner " + listener);
                let idx = this.listeners.indexOf(listener.dispatch);
                if(idx>=0) this.listeners.splice(idx,1);
            });
        }
    }

    /**
     * Signals all listeners that data has been altered
     */
    protected callListeners(): void {

        const listenersCopy = [...this.listeners];
        listenersCopy.forEach(listener => {
           listener({});
        });

    }

[JeanLucPons]

An other idea (I think I will switch to this one) This prevents to call addListener and makes the search in the listener list each time you access the value of a DataSource. This solution implies that the DataSource model is constant and cannot change dynamically (which is my usecase). A dynamic model change can be handled by passing the DataSource array to the dependency list of useEffect (take care that the array reference is stable if models do not change)

/**
 * Hook to register listener to fields when component is mounted and
 * unregister them when component is unmounted
 */
export function useData(atts:Array<DataSource>) : void {

    // Create the dispatcher, React garanty that this function is 
    // stable during the component lifecylce
    const [, _dispatch] = useState({});

    useEffect(() => {
        //Component DidMount
        atts.forEach(a=>{
            a.addListener(_dispatch);
        })
        return () => {
            //Component WillUnmont
            atts.forEach(a=>{
                a.removeListener(_dispatch);
            })
            };
    }, []);

};

Example of usage: Here StateScalar.getValueField() returns one Field of the StateScalar object i want to track

export const MyComponent: FC<{ stateModel: StateScalar} > = ({ stateModel}) => {

  useData([stateModel.getValueField()]);
  let value = stateModel.getValue(); // Do not need to pass the hook here, getValue() is a shortcut to stateModel.getValueField().get()

[TarVK]

That's an interesting suggestion, but I don't think it's a nice fit for this library. In the case of just direct dependencies in react, it may be a good idea, but it doesn't support some of the more advanced features of my system.

You're currently directly adding and removing dependencies on a data source in the second suggestion, but when you have a general data retriever, this would cause much more syntactic overhead and complexity for things like virtual data sources. This is a feature I commonly use, and what I think makes model-react very powerful and flexible. In the first suggestion you don't remove the listener on every update, so a virtual data source like this, wouldn't be stable:

function getSomething(hook?: IDataHook): Something | undefined {
  return isSmthSet(hook) ? getSmth(hook) : undefined;
}

The path that's taken to obtain data here may change per call, so each call will need to register fresh hooks to ensure it's subscribed to all the right dependencies.

I think overall the suggestion is nice in terms of simplicity, but not nearly as flexible as the current system.

[JeanLucPons]

OK So for my usecase, adding and removing dependencies will likely be too much. I have to refresh fast data and refreshers are obviously asynchronous. In my final implementation I think I will also have to add "mutex" mechanism (may be using async-mutex) for the listener lists. So likely not possible for me to add and remove each at each render. Thx ;)

Edit:

My final implementation will probably looks something like this. I used the state to store and returns it. The listeners call the dispatcher with this as argurments. It allows a component that listen on several datasources to know which datasource has tigerred the rendering.

/**
 * Hook to register listener to DataSources when the component is mounted and
 * unregister them when the component is unmounted.
 * Returns the DataSource which has triggered the rendering or undefined
 * when the component is mounted.
 * 
 * Eg: A react component that want to listen on the format and value of a NumberScalar
 * change should use:
 * 
 * export const MyComponent: FC<{ model: NumberScalar }> = ({ model }) => {
 *   let source = useData([model.getValue(),model.getFormat()]);
 *   let value = model.getValue().get()
 *   let format = model.getFormat().get()
 *   if( source === undefined ) {
 *     // The component has just been mounted
 *   } else if ( source == model.getFormat() ) {
 *     // Format has changed
 *   } else
 *   ...
 */

export type DataSourceState = { source: DataSource | undefined }

export function useData(atts: Array<DataSource>): DataSource | undefined {

    // Create the dispatcher, React garanty that this function is 
    // stable during the component lifecycle
    const [source, _dispatch] = useState<DataSourceState>({ source: undefined });

    useEffect(() => {
        //Component DidMount
        atts.forEach(a => {
            a.addListener(_dispatch);
        })
        return () => {
            //Component WillUnmont
            atts.forEach(a => {
                a.removeListener(_dispatch);
            })
        };
    }, []);

    console.log("useData() returned:" + util.inspect(source.source));
    return source.source;

};