Enable metro plugin when expo is installed#1600
Enable metro plugin when expo is installed#1600DaniFoldi wants to merge 2 commits intowebpro-nl:mainfrom
Conversation
commit: |
webpro
left a comment
webpro
left a comment
There was a problem hiding this comment.
Thanks for the PR. Happy to merge, but would be good to have more insights as to how it will behave in practice in real projects.
| const options = await createOptions({ cwd }); | ||
| const { issues, counters } = await main(options); | ||
|
|
||
| assert(!issues.files.has(join(cwd, 'metro.config.js'))); |
There was a problem hiding this comment.
This is fine, but counters.files === 0 so we know there are no unused files.
There was a problem hiding this comment.
Good point, will fix
| 'metro.config.{js,cjs,mjs,ts,cts,mts,json}', | ||
| '.config/metro.{js,cjs,mjs,ts,cts,mts,json}', | ||
| 'package.json', | ||
| ]; |
There was a problem hiding this comment.
| const title = 'Metro'; | ||
|
|
||
| const enablers = ['metro', '@react-native/metro-config']; | ||
| const enablers = ['metro', '@react-native/metro-config', 'expo']; |
There was a problem hiding this comment.
It's bit hard for me to predict the impact of this from this PR alone. Are there maybe public repo/examples that benefit from this, or other useful resources/docs?
There was a problem hiding this comment.
Hmm, for us, the reason for having this file is react-native-svg-transformer, which requires a custom transform being added to metro.
One approach is looking for projects which have a metro config and it contains an expo import https://github.com/search?q=path%3Ametro.config.ts%20%22expo%2Fmetro-config%22&type=code or look for package.json files that have expo and the transformer in their dependencies - this one is easy to add knip to: https://github.com/search?q=path%3Apackage.json+%22%5C%22expo%5C%22%22+%22%5C%22react-native-svg-transformer%5C%22%22+%22%5C%22knip%5C%22%22&type=code
From here, probably a manual knip run to see if it's reported as unused, or a config check to see if it's ignored. What do you think? Happy to do this if you think it's a good approach
There was a problem hiding this comment.
yes that could work, the point is mostly to at least have some confidence this isn't a "local fix" for a single project; apologies the overhead here
having some related work merged in, maybe @wojtekmaj has an opinion?
DaniFoldi
changed the title
3 participants
Hi 👋,
As discussed in #1565, this change enables the metro plugin when expo is detected, and adds a new minimal fixture for this setup.
Also, I saw a tweet recently that the next version of metro would gain support for .ts config files, so I updated the config file detection to support all the formats in https://github.com/facebook/metro/blob/main/packages/metro-config/src/__tests__/loadConfig-test.js - not all projects are going to update immediately (especially expo projects where the version is internally managed), but I assume the number of projects with an existing metro.config.ts file where this would temporarily cause a false negative is probably near 0.