[FOLIO-1694] RFC Routing Refactor Created: 11/Jan/19 Updated: 03/Jun/20 Resolved: 11/Mar/19 |
|
| Status: | Closed |
| Project: | FOLIO |
| Components: | None |
| Affects versions: | None |
| Fix versions: | None |
| Type: | Task | Priority: | P3 |
| Reporter: | John Coburn | Assignee: | John Coburn |
| Resolution: | Done | Votes: | 0 |
| Labels: | RFC, rfc, stripes-force | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original estimate: | Not Specified | ||
| Issue links: |
|
||||||||||||||||||||||||
| Sprint: | stripes-force Sprint 55, stripes-force Sprint 57, stripes-force Sprint 58, stripes-force Sprint 59 | ||||||||||||||||||||||||
| Development Team: | Core: Platform | ||||||||||||||||||||||||
| Description |
|
This JIRA is meant to sum up the different aspects of the routing/url issue that FOLIO faces in order to create a big picture view so that we can choose and describe work to move us to the best path in an iterative way. Browser location/history management.The current pattern typical among core modules is to update the Store via mutator.query.update which, eventually syncs up the url via `history.push` - the problem here is that if the history is updated again through other means (browser back button, subsequent call to history.push) the store is now out of sync, which could cause expected updates to no longer happen and break UI functionality (
mutator.query does provide nice things such as a graceful API in case only a portion of the query parameters need to be updated, but this case should be reduced to a minimum if we're able to rely on the location's path as opposed to query parameters for portions of user workflow that extend beyond the initial record search/preview. Ideally, there's a unidirectional flow between the browser location and state/store of the system, where the location is updated via a link or call to history API and the client state/store/rendering are updated accordingly. Routing re-structure to ui-module codeCurrently, routing is handled in a multitude of ways throughout modules-
Once something changes in the data layer or Routing layer, big refactors will need to take place here and the abstractions that render Routes may be incompatible. Proposed solution to this: Routing and data layer should live at the top level of ui-modules. Data from these connected containers are passed as props into presentational 'view' components, and few things below this view are 'connected' to a data source. URL StructureURL's should contain very little of the current state of the component outside of those necessary for React-Router to render the appropriate view. Need to resolve/document sane usage of query params. Remove Routing in smart-componentsSmart components can function as containers, but should be rendered within separate routes within the modules, but should not supply routes themselves. These will have to be refactored and modules will have to eject from them. For the most part, non-routing code will be split among sub-smart-components which can then be imported by modules, leaving non-rigid use-cases that perform only the heavy-lifting required. As these container components are broken out, The modular pieces can be put to use within their larger molecules - these can still be used in modules for short-term legacy support, but as routing moves to the modules, these molecules will go away as well... Sub-modulesNeed a way to handle routing for these- these may exist as sidecars to detail views. They lean on URL structure. Inter-module navigation (back-links, close buttons, edit buttons, etc)Without manipulating history directly, ui-modules may need to pass appropriate navigational props to 'view' components. These would consist of url's to place as 'href' or 'to'. Some cases where intra-module navigation occur may require some way to hide conditionally hide 'back' links. Nested routesSpike-worthy approach to see if we should employ a nested route solution. May need to build/provide something like eholdings' Route component: https://github.com/folio-org/ui-eholdings/blob/master/src/router.js Routing/query on pluginsPlugins will need further adjustment to work with module routing. |
| Comments |
| Comment by Khalilah Gambrell [ 31/Jan/19 ] |
|
KG : need to determine a new app planned. |
| Comment by Jason Skomorowski [ 12/Feb/19 ] |
Modification to routing re-structureIn meetings stemming from
Deprecate (or replace?) resource-based query paramsIt seems syncing a resource to the query parmeters has been more trouble than it's worth. Should we deprecate this for new development? We can't remove it until we refactor those situations where whole apps get pulled into plugins (eg. plugin-find-user), but we can stop recommending it in documentation. Would we want to replace it with any other helper or are the existing browser and react-router APIs sufficient? |
| Comment by Jason Skomorowski [ 22/Feb/19 ] |
|
So here's the spike on nesting route components---how to keep all our routes in one place and still used react-router. For more detail on the FS approach to nested routes, see Jeffrey's commit here (or also in eHoldings), the rest of the commit/PR provides an example of its use: We could instead use render props and not have to have to maintain this intermediate component and developers won't have to learn anything special outside of core react and react-router. But it's actually a bit more convoluted and so even though the other thing would be particular to FOLIO, it'd be a bit more readable. This isn't... terrible though, conveniently it's a common enough thing one can readily google an example: react-router's own doc provide a third approach which I don't think we should go with but for the sake of completeness: You can make a sort of config object with the whole routing hierarchy and parse it into routes. I don't have a particularly strong opinion beyond not being thrilled with the config object version. If we use render props, we should document the pattern where we mandate routes all be in index.js. If we use the route component, same thing, but now we need to put this component someplace. Should we use stripes-util for that so as to avoid having a dependency on react-router in stripes-components? Does anyone have a strong (or even medium) preference between the first two options? |
| Comment by Jason Skomorowski [ 21/Mar/19 ] |
|
...we wound up opting for the nested route component and are exporting it from core https://github.com/folio-org/stripes-core/pull/648 |