-
Notifications
You must be signed in to change notification settings - Fork 4k
Cleanup platform specifics from hermes build #80110
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
Changes from all commits
9b5297c
e9c217b
4df5ea2
4548e4b
8feec1c
15799a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| const {getDefaultConfig: getExpoDefaultConfig} = require('expo/metro-config'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. expo/metro-config could potentially be removed from the project deps as well?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don’t have this package directly added to package.json, so there’s nothing to remove on our side. We still have some expo packages in the project, it can be required indirectly as a transitive dependency though |
||
| const {getDefaultConfig: getReactNativeDefaultConfig} = require('@react-native/metro-config'); | ||
|
|
||
| const {mergeConfig} = require('@react-native/metro-config'); | ||
|
|
@@ -13,7 +12,6 @@ const envPath = process.env.ENVFILE ? (path.isAbsolute(process.env.ENVFILE) ? pr | |
| require('dotenv').config({path: envPath}); | ||
|
|
||
| const defaultConfig = getReactNativeDefaultConfig(__dirname); | ||
| const expoConfig = getExpoDefaultConfig(__dirname); | ||
|
|
||
| const isE2ETesting = process.env.E2E_TESTING === 'true'; | ||
| const e2eSourceExts = ['e2e.js', 'e2e.ts', 'e2e.tsx']; | ||
|
|
@@ -37,6 +35,7 @@ const config = { | |
| transformer: { | ||
| getTransformOptions: async () => ({ | ||
| transform: { | ||
| experimentalImportSupport: true, | ||
| inlineRequires: true, | ||
| }, | ||
| }), | ||
|
|
@@ -48,6 +47,6 @@ const config = { | |
| : {}, | ||
| }; | ||
|
|
||
| const mergedConfig = wrapWithReanimatedMetroConfig(mergeConfig(defaultConfig, expoConfig, config)); | ||
| const mergedConfig = wrapWithReanimatedMetroConfig(mergeConfig(defaultConfig, config)); | ||
|
|
||
| module.exports = isDev ? mergedConfig : withSentryConfig(mergedConfig); | ||
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.
how about
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 thought about if, but for it we will have to wrap metro config into module to get the
runningInvariable from api.If metro is outside the function, it builds once when the module loads. If we move it inside it rebuilds on every Babel call. TBH I am not sure if it will impact the performance, but it might.
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.
ok