Add expo plugin
#879
Add expo plugin
#879
Conversation
| const resolveEntryPaths: ResolveEntryPaths<ExpoConfig> = async (expoConfig, { manifest }) => { | ||
| const config = 'expo' in expoConfig ? expoConfig.expo : expoConfig; | ||
|
|
||
| let production: string[] = ['index.{js,jsx,ts,tsx}']; |
There was a problem hiding this comment.
Doesn't hurt, but the root index.* pattern is already a default so a bit redundant.
There was a problem hiding this comment.
Good point, I'll remove it here
| .filter(Boolean) as string[]) ?? []; | ||
|
|
||
| const inputs = new Set<Input>([ | ||
| ...['expo', 'react-native', 'react'].map(toProductionDependency), |
There was a problem hiding this comment.
Adds
expo,react, andreact-nativeas dependencies; required for all Expo projects.
Aren't those dependencies referenced in regular ways already? I'd think it isn't necessary to add them explicitly. It's a bit far-fetched, but in theory this might change, and if that's what they are, peer dependencies should picked up properly already.
There was a problem hiding this comment.
Ahh I didn't realize Knip picked up peer dependencies. Very cool! That takes care of react and react-native, and on second thought expo will almost certainly be in the package.json scripts (if not imported in the project), so I'll remove that as well.
|
|
||
| // https://docs.expo.dev/router/installation/#setup-entry-point | ||
| if (manifest.main === 'expo-router/entry') { | ||
| production = ['app/**/*.{js,jsx,ts,tsx}', 'src/app/**/*.{js,jsx,ts,tsx}']; |
There was a problem hiding this comment.
Why does this manifest.main value result in all those files becoming entry files, isn't that pattern too broad?
There was a problem hiding this comment.
Setting main to expo-router/entry essentially "turns on" expo-router (if the package is installed, of course). It's a file-system based router, so every file in the app directory becomes it own route. (e.g. an app/profile/settings.tsx file will become a /profile/settings URL) This is all done under the hood by expo-router; these route files are not imported elsewhere in the project source code. So I do think the broad pattern is warranted here, you can name a file anything you want, and expo router will pick it up. It's the same concept as the NextJS "pages" router; anything in that directory is an entry file.
There was a problem hiding this comment.
Side note, the app directory can live either at the root or under src, and src/app takes precedence. (https://docs.expo.dev/router/reference/src-directory/) Is it worth it to check for the existence of src/app and fall back to app if it doesn't exist, instead of using both patterns at the same time?
There was a problem hiding this comment.
Clear, thanks for explaining that. I'd say the check isn't really necessary, assuming that globbing will essentially do the same thing. Unless you can think of a situation where this might go awry, i guess it makes sense to keep this simple.
There was a problem hiding this comment.
Sounds good, I'll keep it simple
There was a problem hiding this comment.
i also use expo-router but in a index.ts file as my entry and it seems that break that plugin, i can't get it to not put everything as unused. maybe i configure it badly.
one of the reason for using index.ts as entry file even with expo router is for example skia
https://shopify.github.io/react-native-skia/docs/getting-started/web/
There was a problem hiding this comment.
Thanks @wcastand, here as a comment it might get lost, any chance you could give more details so we have something to look into? What's the issue you're seeing and how to reproduce it?
There was a problem hiding this comment.
i changed the way i handle the entry point so seems to be good for now, issue i can see seems to be with custom modules in modules directory being flagged at unused.
other than that hard to say what's an issue on our repo or not so will take a deeper look and if i find anything i will get back to you.
great tool either way thanks :)
| inputs.add(toProductionDependency('expo-navigation-bar')); | ||
| } | ||
|
|
||
| return [...inputs]; |
There was a problem hiding this comment.
That looks like a lot of magic Expo is pulling off, thanks a lot for figuring that all out here. Also appreciate you've added links to docs and used some "heuristics" to best-guess including things like expo-system-ui.
commit: |
|
Excellent stuff, Jonathan! Really appreciate this, and I'm sure others will as well. |
|
🚀 This pull request is included in v5.41.0. See Release 5.41.0 for release notes. Using Knip in a commercial project? Please consider becoming a sponsor. |
|
Thanks for the awesome project, it's tremendously useful! There will be more plugin PRs from me :) |
|
@jjjjonathan Thanks for this plugin! It allowed me to remove a lot of whitelisted dependencies. I do have a few left, that might still be relevant:
|
|
@jerone Glad you're getting some use out of it! Here are my thoughts/suggestions:
|
This PR adds a plugin for Expo.
Features
expo-router(A file-based router akin to NextJS) is used or not.Addsexpo,react, andreact-nativeas dependencies; required for all Expo projects.expo-dev-clientwill cause Expo to use the package when building the native app; it does not need to referenced or imported anywhere other than thepackage.json.