-
Notifications
You must be signed in to change notification settings - Fork 147
Add expo plugin #1271
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
base: main
Are you sure you want to change the base?
Add expo plugin #1271
Conversation
🦋 Changeset detectedLatest commit: 7341e40 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for GitHub.
|
5d4d03b
to
140d71f
Compare
140d71f
to
216ab15
Compare
Hey @ceopaludetto, absolutely, go for it 👍 |
38e49a2
to
f5bc4ef
Compare
f5bc4ef
to
d28352d
Compare
d28352d
to
674306a
Compare
674306a
to
07e0c75
Compare
07e0c75
to
1af629c
Compare
@jbroma added a new expo guide in the docs and also added the tester application. We can combine both |
I'm tracking the progress in this PR, very interested on it. I just wondering if would be possible to launch just with expo instead of react-native cli since it wouldn't require to manage pods locally or any kind of build just fetching a development build from expo cloud and run it using the repack bundler. |
1af629c
to
7341e40
Compare
Hey, thanks for the interest! The main reason we switched from expo CLI to React Native CLI is that when you run something like |
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.
hey @ceopaludetto sorry for the delay with the review! I found few things we need to address but the rest looks good 👍
one more thing: when doing prebuild + run-ios I kept running into issues because I don't have cocoapods globally installed and use local ruby setup - perhaps we could fix that with Gemfile + Gemfile.lock like in other testers? (or maybe there is a nicer fix for that). Prebuild + android worked fine for me, good job with the tester 👍
"node": ">=18" | ||
}, | ||
"peerDependencies": { | ||
"@callstack/repack": "workspace:*", |
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.
lets define those explicitly as a starting point for this plugin 👍
from my experience its best to define versions with open upper boundary, like >=53.0.0
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.
Ran into some issues here when building Android, I think we are missing from fields like in the other tester app
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.
shouldn't we use registerRootComponent
from expo
here?
// Alias both react and react-native to ensure that we don't end up with multiple versions | ||
// of these libraries in the bundle. | ||
// | ||
// This is needed in these monorepo setups where there are multiple copies of react | ||
// and react-native in the node_modules. | ||
react: require.resolve('react'), | ||
'react-native': require.resolve('react-native'), |
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.
is this an issue here with pnpm? Sounds like this should happen in setups with hoisting 🤔
}, | ||
module: { | ||
rules: [ | ||
...Repack.getJsTransformRules(), |
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.
lets use babel-swc-loader
here instead (take a look at config for other tester)
new compiler.webpack.DefinePlugin({ | ||
'process.env.EXPO_PROJECT_ROOT': JSON.stringify(root), | ||
// If Expo Router is enabled, pass additional environment variables | ||
...(router | ||
? { | ||
'process.env.EXPO_BASE_URL': JSON.stringify(router.baseUrl), | ||
'process.env.EXPO_ROUTER_ABS_APP_ROOT': JSON.stringify(router.root), | ||
'process.env.EXPO_ROUTER_APP_ROOT': JSON.stringify(router.root), | ||
'process.env.EXPO_ROUTER_IMPORT_MODE': JSON.stringify('sync'), | ||
} | ||
: {}), | ||
}).apply(compiler); |
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.
I think its cleaner to split it into two DefinePlugin
if ( | ||
compiler.options.mode === 'development' && | ||
!!compiler.options.devServer | ||
) { | ||
compiler.options.devServer.proxy ??= []; | ||
|
||
// Redirect `/.expo/.virtual-metro-entry` to `/index` to match metro behavior in Expo managed projects | ||
compiler.options.devServer.proxy.push({ | ||
context: ['/.expo/.virtual-metro-entry'], | ||
pathRewrite: { '^/.expo/.virtual-metro-entry': '/index' }, | ||
}); | ||
} |
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.
this has no effect because devServer options are set before plugin apply
runs - there is no way to inject devServer options through the plugin.
An alternative to that would be to wrap the entire user configuration and inject it there, kinda like Rozenite does it here
Summary
Test plan
Todos