Expose setprops #2758
Expose setprops #2758
Conversation
| notifyObservers({id: componentId, props}) | ||
| ); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The "observer" pattern in this redux context is very weird, it's meant to act as reducer with side effects.
I think this will be called on every callbacks and it should be only defined once. Maybe just move the definition on the window to store.ts in createApp instead would be better.
There was a problem hiding this comment.
Within this context (line 58 store.ts):
private createAppStore = (reducer: any, middleware: any) => {
this.__store = createStore(reducer, middleware);
this.storeObserver.setStore(this.__store);
this.setObservers();
};
… with `createAppStore`
… a list of stores that the app has
…or using `setProps`
| const set_props = (updates: []) => { | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| const ds = (window.dash_stores = window.dash_stores || []); | ||
| for (let y = 0; y < ds.length; y++) { | ||
| const {dispatch, getState} = ds[y]; | ||
| const {paths} = getState(); | ||
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| updates.forEach(({id, ...props}) => { |
There was a problem hiding this comment.
I think the api for this could be simpler without the batching of updates and extracting the id from the props into a function argument.
The function signature would then look like:
set_props(id: string | object, props: {[k: string]: any}): void;
There was a problem hiding this comment.
Maybe, unless we would want to be able to send batches of updates and not trigger the renderer until the props have been updated?
There was a problem hiding this comment.
For batching to work it needs a import {batch} from 'react-redux', or in react 18 it would be kinda automatic in effects and lifecycle methods. But it's only for calling within a react context, which might not always be the case here.
There was a problem hiding this comment.
Yeah, for this, it would be in the context of JS, however, I thought with the function coming from within the React lifecycle, this would still be a react response?
There was a problem hiding this comment.
Inside a clientside callback there is a react context, but since this is exposed globally it may be called from event handlers or anywhere really.
I think in the most simple cases it would be to update a single component like a toast message and multiple props on the component can still be set at once.
This API will also be implemented on the Python side for updating non output props inside callbacks, a simpler API will be better in that case and to make sure the id is provided.
There was a problem hiding this comment.
Ok, yeah, I can change it, was just thinking in general how the callbacks work with being able to pass multiple at the same time.
| // eslint-disable-next-line @typescript-eslint/ban-ts-comment | ||
| // @ts-ignore | ||
| const ds = (window.dash_stores = window.dash_stores || []); |
There was a problem hiding this comment.
My TypeScript skills are pretty basic, but can't we avoid these comments (3 in this file and store.ts) with a few well-placed as any or some such? @T4rk1n you're better at this than I am, can you comment?
This PR is an attempt to expose the Dash components and renderer to be utilized by JS scripts/components by bringing the
setPropstodash_clientside.setProps.Contributor Checklist
setPropstodash_clientside.setPropsoptionals
CHANGELOG.md