-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(@netlify/build-info): add React Router 7 detection #5930
base: main
Are you sure you want to change the base?
Conversation
bfe95a3
to
d6c7af4
Compare
d6c7af4
to
820bd41
Compare
Technically we'd want to add it to Vite's `excludedNpmDependencies`, but we actually only want to exclude RR >=7. Many RR <7 users will be using Vite, so we don't want to break that. But everything works fine if we just don't do anything here, since frameworks get priority over bundlers when there are multiple matches. (I'm not really sure why we both excluding Vite frameworks in the Vite logic).
820bd41
to
a75b85a
Compare
readonly id = 'react-router' | ||
name = 'React Router' | ||
configFiles = ['react-router.config.ts', 'react-router.config.js'] | ||
npmDependencies = ['react-router'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to detect the plugin we built as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. We've been doing that for some of the frameworks, but I was thinking about it and couldn't come up with any reason for it, so I didn't include it. You definitely need react-router to use react-router; checking for additional dependencies won't help.
Summary
React Router 7 was released a couple weeks ago. It's sort of the next version of Remix and also sort of the next version of React Router. It can be used as a library or as a framework.
Thus, this adds it to our framework detection, but only for version 7+.
There are some oddities here:
@react-router/dev
as an almost certain hint that the site uses RR7 as a framework, but according to the Remix/RR team this may not hold forever. Seems OK for now.excludedNpmDependencies
, but we actually only want to exclude RR >=7 (many RR <7 users will be using Vite or something else, so we don't want to break that).