Memory routing to fix JupyterLab extension

[gabalafou]

PR #389 broke the JupyterLab extension. Before 389, the app router was configured to route any URL to the main React component. Essentially there wasn't any URL routing in the app. Which also meant that there also weren't any possible 404 Not Found pages. After 389, the app router was configured to match specific routes. But when Conda Store UI is loaded inside of JupyterLab the URLs in the browser address bar are meant to be consumed by JupyterLab. Since the Conda Store UI app did not know how to handle the URL path /lab this resulted in the JupyterLab extension showing a kind of 404 page.

The solution is to allow the app to be configured to not rely on the browser location for routing. (This is similar to how it would work if it were programmed as a mobile app.)

Fortunately, the router that we use, React Router, already provides an in-memory router. (This router is most commonly used in Node.js testing environments, but it works as a general-purpose router that does not rely on the URL in the browser.)

Description

This pull request:

• Adds new app config var: routerType (REACT_APP_ROUTER_TYPE)
  • type: string with two possible values ["memory", "browser"]
• Uses the value of the new config var to create memory-based router or (default) one that uses the browser's history API.
  • See: createMemoryRouter

Pull request checklist

• Did you test this change locally?
• Did you update the documentation (if required)?
• n/a Did you add/update relevant tests for this change (if required)?

How to test locally

Load the app and make sure that you can click around and go forward and backward in the browser.

To test that this PR fixes the extension, you can follow the release instructions to create a local tarball - for example at /dev/conda-store-ui-389.tgz. Then modify the entry for @conda-store/conda-store-ui in the jupterlab-conda-store package.json to point to /dev/conda-store-ui-389.tgz. Next, find where the App class from conda-store-ui is imported and make sure to pass routerType: "memory" to it - see conda-incubator/jupyterlab-conda-store#63.

[peytondmurray]

Cool, I didn't know this was a thing you can do. Seems like the best option when JupyterLab is already making use of the URL.

This seems like a long shot but I have to ask: is there any way we can have a test which checks a few routes to make sure that the extension and the standalone app itself each works with the separate routing methods? I get that the extension should test itself, but in a sense the extension is downstream, and I guess my question is about ensuring we don't break downstream stuff in the future.

This looks good, but I haven't tested this locally, but will do so now.

[gabalafou]

is there any way we can have a test which checks a few routes to make sure that the extension and the standalone app itself each works with the separate routing methods?

This is a great suggestion. I interpret it as writing two separate tests, one that uses the browser (history API) router and one that uses the memory router. The browser router test would check that the app can be loaded at a page in the app that is not the start page and that a link can be followed to another page, perhaps also it would check that a call to pushState puts the app in the right state, i.e. at the right page. The memory router test would check that the home page of the app loads even if the browser is at a URL that the app doesn't recognize and that a link can be followed to another page.

I can try to write these tests. It will probably be easiest to add them in the Playwright tests because the Jest tests run in a Node.js environment, and to test the browser router against the memory router, we need a browser environment.

But just be aware that releasing a new version of the JupyterLab extension is blocked until this PR is merged and we make a new release of Conda Store UI. So I wonder if I should merge this in now, and add the tests afterwards as soon as possible.

[trallard]

is there any way we can have a test which checks a few routes to make sure that the extension and the standalone app itself each works with the separate routing methods?

This is a great suggestion. I interpret it as writing two separate tests, one that uses the browser (history API) router and one that uses the memory router. The browser router test would check that the app can be loaded at a page in the app that is not the start page and that a link can be followed to another page, perhaps also it would check that a call to pushState puts the app in the right state, i.e. at the right page. The memory router test would check that the home page of the app loads even if the browser is at a URL that the app doesn't recognize and that a link can be followed to another page.

I like this idea incredibly, and it is worth adding those tests.

But just be aware that releasing a new version of the JupyterLab extension is blocked until this PR is merged and we make a new release of Conda Store UI. So I wonder if I should merge this in now, and add the tests afterwards as soon as possible.

Let's do this. We are overdue a release.

[trallard]

Since you are adding a new configuration, would it be worth updating the docs? https://conda.store/conda-store-ui/how-tos/configure-ui

[gabalafou]

Yes, if we're happy with this, I will update the docs

[trallard]

Seems fine, it would appear that the memory setting would only be needed for the JLab extension right?

[gabalafou]

Yes, it would be needed for the extension or any context in which the app is not supposed to read or manipulate the URL in the browser address bar

[trallard]

Ok I will approve this, but will need an update on the docs.

[gabalafou]

I put the tests in a separate PR because they required a significant change to the React app code, but one that I think is actually also a bug fix:

• Add React router tests gabalafou/conda-store-ui#1

[trallard]

Ok I will approve this, but will need an update on the docs.