Upgrade to React Router v7 #4929

sergei-maertens · 2024-12-16T15:06:02Z

We're on v6 with some leftovers from v5, so let's do this before it becomes too painful.

In particular, we need to re-organize a bunch of code to not use the JSX nested routes, and convert everything to (static) routes with elements. This will likely mean moving some submission checks to context & hooks instead of explicit props. Possibly we can tap into the loaders/actions too for better DX.

Steps

Upgrade to v6 with all future flags enabled
Upgrade to v7

Moved component into its own file so in the future we can more easily remove it, and to declutter index.jsx from a package entrypoint. Next, we can export the nested routes from the CoSign package without adding even more weight/content to the file itself in terms of implementation.

…s to routes object State/props are moved into a context provider that wraps around the outlet, taking care of the data loading. Once we're no longer the <Routes> approach, we can investigate what we can do with loaders to pass information/state around.

…omponent (start) The top-level component is responsible for some state management while the child route ./start actually ensures the right component gets rendered. Setting up the tests like this gets the boilerplate out of the way and allows us to test from the user perspective for the more complicated stuff in the cosign component.

…ssion from API

Added test to ensure that the check route with a submission ID in the query params results in the submission summary being disabled for verification.

… require any props Split the state up in the components used to render each route/path, which allows localizing the state. For the synchronization between routes, React context is used.

Moved component into its own file so in the future we can more easily remove it, and to declutter index.jsx from a package entrypoint. Next, we can export the nested routes from the CoSign package without adding even more weight/content to the file itself in terms of implementation.

…s to routes object State/props are moved into a context provider that wraps around the outlet, taking care of the data loading. Once we're no longer the <Routes> approach, we can investigate what we can do with loaders to pass information/state around.

…omponent (start) The top-level component is responsible for some state management while the child route ./start actually ensures the right component gets rendered. Setting up the tests like this gets the boilerplate out of the way and allows us to test from the user perspective for the more complicated stuff in the cosign component.

…ssion from API

Added test to ensure that the check route with a submission ID in the query params results in the submission summary being disabled for verification.

… require any props Split the state up in the components used to render each route/path, which allows localizing the state. For the synchronization between routes, React context is used.

The props are moved to the parent context instead so that the component itself can be statically declared in the routes array.

Moved component into its own file so in the future we can more easily remove it, and to declutter index.jsx from a package entrypoint. Next, we can export the nested routes from the CoSign package without adding even more weight/content to the file itself in terms of implementation.

…s to routes object State/props are moved into a context provider that wraps around the outlet, taking care of the data loading. Once we're no longer the <Routes> approach, we can investigate what we can do with loaders to pass information/state around.

…omponent (start) The top-level component is responsible for some state management while the child route ./start actually ensures the right component gets rendered. Setting up the tests like this gets the boilerplate out of the way and allows us to test from the user perspective for the more complicated stuff in the cosign component.

…ssion from API

Added test to ensure that the check route with a submission ID in the query params results in the submission summary being disabled for verification.

… require any props Split the state up in the components used to render each route/path, which allows localizing the state. For the synchronization between routes, React context is used.

The props are moved to the parent context instead so that the component itself can be statically declared in the routes array.

Forgot we use spec rather than test.

Moved component into its own file so in the future we can more easily remove it, and to declutter index.jsx from a package entrypoint. Next, we can export the nested routes from the CoSign package without adding even more weight/content to the file itself in terms of implementation.

…s to routes object State/props are moved into a context provider that wraps around the outlet, taking care of the data loading. Once we're no longer the <Routes> approach, we can investigate what we can do with loaders to pass information/state around.

…omponent (start) The top-level component is responsible for some state management while the child route ./start actually ensures the right component gets rendered. Setting up the tests like this gets the boilerplate out of the way and allows us to test from the user perspective for the more complicated stuff in the cosign component.

…ssion from API

Added test to ensure that the check route with a submission ID in the query params results in the submission summary being disabled for verification.

… require any props Split the state up in the components used to render each route/path, which allows localizing the state. For the synchronization between routes, React context is used.

The props are moved to the parent context instead so that the component itself can be statically declared in the routes array.

Forgot we use spec rather than test.

…nning of a form * Navigate from the start to introduction page and first step * Navigate from start directly to first step when no introduction page is used.

In v5, this hook probably didn't exist yet and that's why we added it, but v6 has a convenience hook already. Our own implementation now simply wraps it, and we can deprecate it at some point and use react router's hook instead, as it also comes with a setter.

…nce implementation into hook Rather than sprinkling the param processing through various parts of the code base and repeat the implementation, provide a custom hook that encapsulates the implementation details. The hook reads the reference from the query params, and provides a helper to mutate the reference in (derived) react houter URLs. Note that the window.location.origin as base is just a workaround to be able to use the `URL` constructor and object for query string manipulation. The actual real window.location is irrelevant, as that state is managed by react router. We only care for the path (or rather the path segment) that may have some query params attached to it.

… into separate component Created a separate component that handles the redirect logic from the landing page, using the context rather than props to get the necessary information. This makes it possible to use the component as element in a statically defined route.

…tatic route definitions Created a static route definitions file for routes nested under the Form component. They are rendered in the specified outlet node, and included in the main app routes definition. The rest of the <Route> elements will follow in separate commits.

…nent to require no props Instead of passing the extra parameters, we extract the initial data reference using the hook from the LandingPage component refactor.

Update the tests to prepare for refactors in the implementation. * Wrap the component in a proper data router instead of the JSX/component router. * Replace the useQuery mocks with actual locations/url information that contains the parameters so we can use the real react-router behaviour and don't need the mocks in the first place, which reduces tight coupling.

…m query instead of prop The FormStart component now grabs the initial data reference directly from the URL parameters rather than requiring the parent component needing to do this and pass it down as a prop. This is a first step towards not needing to specify props at all to FormStart so that it can be included in the data router routes (statically).

…ead of prop in FormStart We wrap the app in form context anyway when the SDK is initialized, as the form is statically loaded only once initially when the SDK bootstraps. This means we do not need to pass it as a prop, nor do we need to worry about too-frequent re-renders.

…ur mock functions The mock functions should at least be easier to centrally update, while we still have the ability to override/specify the properties that are relevant to the test/behaviour being tested.

The app debug state is now displayed on the session-expired page/route, which can give some more insight on what happens with the session reset etc. Additionally, when the expiry date is reset it's set to null, and the delta calculations are off/weird because of Javascript doing math on 'null' objects. Now, we display a dash to indicate there's no session expiry at all. This was discovered during testing if certain code is still required or not.

…t into own route The component is refactored with the goal of configuring it in a static route definitions file, which means it cannot take any dynamic props as input as that is not statically available. The most important props are the current submission in the session, initial data reference and callbacks for when a submission is created or session stopped (because the user logs out, for example). For the initial data reference, we can use the hook from earlier refactors to grab it directly from the query parameters. The submission and relevant callbacks are now provided by the container/wrapper Form component that manages the submission state, through context. We have to be careful here to properly memoize callbacks as there's quite a substantial risk of running into infinite re-renders locking up the browser. Hopefully in the future we can remove those footguns by using loaders and actions of the react-router library. The core onFormStart callback is hereby also moved from the Form component to the FormStart component which properly localizes the related behaviours. We only send the created submission back up to the parent state when the creation is done. Additionally, I've opted to use 'named arguments' for the anonymous flag to make the code easier to read/understand. Secondly, the browser event doesn't need to be passed to the callback, since the form submit event is already suppressed when the callback is invoked. Finally - the previous version of this code would also reset the session expiry whenever a submission is started. This was required to get rid of the session expired error message. This had become obsolete when the session expiry was moved to its own route and component, which already resets the session automatically. No expiry messages are displayed when you navigate away from the expiry notice to the start page by clicking the restart link.

… tests Made the tests more robust by doing proper URL parsing instead of string comparisons, as the order of query string parameters is not important or deterministic. The router wrapper is updated so we can pass the current location without monkeypatching the global window object, which also cleans up the test a bit more. Finally, the onFormStart callback is updated since we don't pass an event any longer - it is handled on the form submit action already.

Updated the existing tests for the new component structure and injection of dependencies, and some tests were moved from Form to FormStart tests because the behaviour being tested was also moved. Less mocks are now used and the tests should generally be a bit more robust.

sergei-maertens added topic: frontend triage Issue needs to be validated. Remove this label if the issue considered valid. enhancement topic: tech debt labels Dec 16, 2024

sergei-maertens added this to the Release 3.1.0 milestone Dec 16, 2024

sergei-maertens added owner: dimpact and removed triage Issue needs to be validated. Remove this label if the issue considered valid. labels Jan 6, 2025

sergei-maertens added this to Development Jan 6, 2025

github-project-automation bot moved this to Todo in Development Jan 6, 2025

sergei-maertens self-assigned this Jan 6, 2025

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 7, 2025

🤡 [open-formulieren/open-forms#4929] Mock retrieving summary of submi…

ff93b9d

…ssion from API

sergei-maertens mentioned this issue Jan 7, 2025

Convert cosign v5 react-router routes to data router format open-formulieren/open-forms-sdk#769

Merged

sergei-maertens moved this from Todo to In Progress in Development Jan 8, 2025

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 8, 2025

🤡 [open-formulieren/open-forms#4929] Mock retrieving summary of submi…

bdf214e

…ssion from API

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 8, 2025

✅ [open-formulieren/open-forms#4929] Fix story args

50a59b8

The props are moved to the parent context instead so that the component itself can be statically declared in the routes array.

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 8, 2025

🤡 [open-formulieren/open-forms#4929] Mock retrieving summary of submi…

2b7ce5e

…ssion from API

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 13, 2025

🚚 [open-formulieren/open-forms#4929] Fix test filename

88bf5c9

Forgot we use spec rather than test.

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 13, 2025

🤡 [open-formulieren/open-forms#4929] Mock retrieving summary of submi…

8004928

…ssion from API

sergei-maertens added a commit to open-formulieren/open-forms-sdk that referenced this issue Jan 13, 2025

🚚 [open-formulieren/open-forms#4929] Fix test filename

69651d2

Forgot we use spec rather than test.

sergei-maertens closed this as completed in open-formulieren/open-forms-sdk#769 Jan 13, 2025

github-project-automation bot moved this from In Progress to Done in Development Jan 13, 2025

sergei-maertens reopened this Jan 13, 2025

github-project-automation bot moved this from Done to In Progress in Development Jan 13, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to React Router v7 #4929

Upgrade to React Router v7 #4929

sergei-maertens commented Dec 16, 2024

Upgrade to React Router v7 #4929

Upgrade to React Router v7 #4929

Comments

sergei-maertens commented Dec 16, 2024