dojo / framework

Dojo Framework. A Progressive Framework for Modern Web Apps
https://dojo.io/
Other
585 stars 78 forks source link

Auto-register `path()` calls in provider renderer #332

Closed aciccarello closed 5 years ago

aciccarello commented 5 years ago

Enhancement Currently providers require a paths property to tell it what paths to listen to. StoreProviders should listen to what path calls occur in the renderer and auto-register those paths to ensure the renderer always matches the store state. If a path property is defined, this functionality would be disabled to allow fine-tuning which state changes trigger a re-render.

Motivation It is easy to forget to update this list which can lead to bugs where, depending on fetch response order, a provider may or may not re-render (due to a missing registered path)

Package Version: 5.0.3

Code

Expected behavior:

<StoreProvider
    stateKey="state"
    renderer={(store) => {
        const { get, path } = store;
        // Both of these paths are watched automatically
        const todos = get(path('todos', 'data'));
        const isLoading = get(path('todos', 'isLoading'));

        if (!isLoading && typeof data === 'undefined') {
            fetchTodos(store)({});
        }

        return <Todos data={todos} isLoading={isLoading} />;
    }}
/>

Actual behavior:

<StoreProvider
    stateKey="state"
    paths={(path) => [path('todos', 'data')]} // Oops, forgot to watch isLoading
    renderer={(store) => {
        const { get, path } = store;
        const todos = get(path('todos', 'data'));
        const isLoading = get(path('todos', 'isLoading'));

        if (!isLoading && typeof data === 'undefined') {
            fetchTodos(store)({});
        }

        return <Todos data={todos} isLoading={isLoading} />;
    }}
/>
agubler commented 5 years ago

This is awkward with the existing StoreProvider and to do properly would be a breaking change. However the new store middleware that will be available in Dojo 6 auto registers widgets to sections of state that are returned using the middleware's get method. This will be the recommended approach for working with stores reactively in a render function along with the use of functional widgets. Closing this for now as we are unlikely to add this support to StoreProvider as a result.