web-client-ui
web-client-ui copied to clipboard
Clean up dependencies for @deephaven/redux
@deephaven/redux should not have any dependencies on other @deephaven packages. Essentially it just needs the middleware stuff and registry, and the rest should be actions/selector exported by other packages.
E.g. The setCommandHistoryStorage action should be registered by the @deephaven/dashboard-core-plugins (specifically would be @deephaven/console-plugin if we had that).
I think it's fine for redux to reference types from other deephaven packages, but we don't want it the other way around where anything that isn't the main app (or dashboard plugins maybe) referencing redux. Really it depends on how we want to separate it
- Redux references packages, packages don't reference redux
I think this would be fine because the redux store is an app specific implementation. You can pull in chart without redux and it should work just fine. Any redux connect/middleware/updates should be handled at the app level (or dashboard core plugins) which then just delegates the updated state to the component which does not know about redux
- Packages reference redux, redux doesn't reference packages
This would couple our packages to redux which I think is the wrong move.
- Neither references each other
This could also be fine and might be the safest overall. Involves a bit of duplication of types, but also prevents accidentally changing types in a package and not realizing it modified the redux store or workspace storage.
Since we have migrations for workspace data, option 3 is probably what we want? Then if something in chart gets modified, TS should warn us if redux expects a different type instead of relying on the type from chart
It's come to light that @deephaven/redux is more of an app level package, so it should be fine if it depends on app packages. Types shouldn't necessarily be defined in @deephaven/redux. Other deephaven packages should not depend on redux.
One thought I've had towards cleaning up redux in general is making the @deephaven/redux package just utils (and maybe naming @deephaven/redux-utils or base or something) and not any selectors/actions/reducers.
Then each package can have its own selectors/actions/reducers which use the @deephaven/redux-utils package to register them (like we do now). In each package (like dashboard as an example), we would essentially create what redux toolkit calls a "slice". Anything that needs to work with that slice would need to import the selectors/actions from the specific package anyway.
Then inside code-studio we have the store setup and any other app specific redux. Any redux that is shared between DHC and DHE would be in a non-app level package. Then we still have a central-ish definition of the store (in the app package) without the current difficulty of defining the shape of the whole store without proper types in a lot of places (like workspace and dashboard data are mostly unknowns we cast when selecting)