feat(remix): Add dynamic loading of @remix-run/router module.
#10458
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size-limit report 📦
|
AbhiPrasad
reviewed
Feb 1, 2024
| try { | ||
| pkg = await import(moduleName); | ||
| } catch (e) { | ||
| pkg = await import(`${cwd()}/node_modules/${moduleName}`); |
There was a problem hiding this comment.
this doesn't work in CJS right? Can you confirm that?
There was a problem hiding this comment.
I have done a little research, but could not produce a case where this breaks. It looks like it works fine on both CJS and ESM. Not in worker environments though.
But, while checking the previous discussions (#5897), the main reason we need this was our TS version and it's up to date now, so we can just try making @remix-run/router a dependency and stop doing this anyways? WDYT?
There was a problem hiding this comment.
so we can just try making @remix-run/router a dependency and stop doing this anyways? WDYT
Let's do that instead!! Less dynamic imports the better.
AbhiPrasad
pushed a commit
that referenced
this pull request
Feb 21, 2024
Backports #10479 to v7 branch Original PR Description: > Fixes: #10349 > Related: #5860 > Related: #10458 > > Removes dynamic loading of `react-router-dom` and makes `@remix-run/router` a peer dependency. > > We don't need to dynamically load `react-router-dom` as our TypeScript version is now up-to-date.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Potentially fixes: #10349
Added dynamic loading support for
@remix-run/routerpackage as a provider ofmatchRoutes.matchRoutesfromreact-router-domand@remix-run/routercan be used interchangeably for our use case.Also updated integration tests' Remix version to
1.19.3which is the latest1.xversion.Don't have the reproduction of the original issue, but I expect this PR to resolve that problem.