search

IjzerenHein / react-native-bundle-visualizer

See what packages are increasing your react-native bundle size 📦
MIT License
1.46k stars 44 forks source link

chore: update `@expo/metro-config` to match SDK 42 #64

Closed Simek closed 3 years ago

Simek commented 3 years ago

This PR updates @expo/metro-config version to match the SDK 42 ABI version (^0.1.70):

This bumps also cleans up the package dependencies and after the new release will help to remove old dependencies from Expo workspace - react-native-bundle-visualizer is used in home app.

Between 2.2.1 and 2.3.0 the @expo/metro-config dependency has been locked to certain version (not sure if it was intentional) which results in resolving old dependencies - https://github.com/IjzerenHein/react-native-bundle-visualizer/commit/273af8ac86af72bcd312c386ce4ff1a99737b2f9.

IjzerenHein commented 3 years ago

I had tried upgrading that dependency but was getting errors. It starts failing with the following error when running the Expo36 test:

TypeError: Cannot read property 'length' of undefined
    at /Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:295:27
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:52:24)
    at _next (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:72:9)
    at /Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:77:7
    at new Promise (<anonymous>)
    at /Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:69:12
    at _applyAssetDataPlugins (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:308:33)
    at applyAssetDataPlugins (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:284:33)
(node:47140) UnhandledPromiseRejectionWarning: Error: Command failed with exit code 1: ./node_modules/.bin/react-native bundle --platform ios --dev false --entry-file ./node_modules/expo/AppEntry.js --bundle-output /var/folders/j5/jr7vcj9d0p72_725gtw3bh6m0000gn/T/react-native-bundle-visualizer/Expo36/ios.bundle --sourcemap-output /var/folders/j5/jr7vcj9d0p72_725gtw3bh6m0000gn/T/react-native-bundle-visualizer/Expo36/ios.bundle.map --config /Users/hein/repos/Hein/react-native-bundle-visualizer/src/expo-metro.config.js
EXPO_TARGET is deprecated. Learn more: http://expo.fyi/expo-extension-migration
jest-haste-map: Watchman crawl failed. Retrying once with node crawler.
  Usually this happens when watchman isn't running. Create an empty `.watchmanconfig` file in your project's root folder or initialize a git or hg repository in your project.
  Error: Watchman error: unable to resolve root ./: path "./" must be absolute. Make sure watchman is running for this project. See https://facebook.github.io/watchman/docs/troubleshooting.html.
error assets/images/robot-dev.png: Cannot read property 'length' of undefined. Run CLI with --verbose flag for more details.
Loading dependency graph, done.
TypeError: Cannot read property 'length' of undefined
    at /Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:295:27
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:52:24)
    at _next (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:72:9)
    at /Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:77:7
    at new Promise (<anonymous>)
    at /Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:69:12
    at _applyAssetDataPlugins (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:308:33)
    at applyAssetDataPlugins (/Users/hein/repos/Hein/react-native-bundle-visualizer/test/Expo36/node_modules/metro/src/Assets.js:284:33)
    at makeError (/Users/hein/repos/Hein/react-native-bundle-visualizer/node_modules/execa/lib/error.js:60:11)
    at handlePromise (/Users/hein/repos/Hein/react-native-bundle-visualizer/node_modules/execa/index.js:118:26)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
(node:47140) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:47140) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
Simek commented 3 years ago

Ohh, I see. Maybe the Expo36 example could be binded to lower version of react-native-bundle-visualizer (2.2.1 or 2.3.0) or this update should be released as new major version (3.x) which requires RN X and Expo X and the older examples should stick to 2.x branch?

Expo 36 has been released 1,5 years ago (https://blog.expo.dev/expo-sdk-36-is-now-available-b91897b437fe), and support has been dropped a while ago (https://github.com/expo/expo/pull/11033), currently it looks like SDK 39 is the lowest supported version.

Simek commented 3 years ago

After updating example locally to Expo 39 and using default metro version or pinning metro dependency to higher version that default, I'm still receiving the same error. 🙁 It looks like dropping the Expo test/support for 36-39 will be required, to bump @expo/metro-config.

It looks like this one is related to issue described in this PR in Metro repository:

I can confirm that Expo 42 test is working correctly with the updated metro-config.

IjzerenHein commented 3 years ago

Thanks for looking into this @Simek !

As of Expo SDK 41, it is no longer needed to use the --expo command and the provided expo-metro.config.js. So it doesn't make sense to upgrade @expo/metro-config when it is no longer needed, as it would break compat with older versions.

Instead, I suggest removing @expo/metro-config entirely and deprecating the --expo bare/managed command-line option. And releasing that as a major new version (3.x). And to include a version table in the README outlining it's compatibility with older versions.

What do you think of that?

Simek commented 3 years ago

That sounds like great plan, thank you for the explanation! 👍 I didn't know that metro-config is no longer required since SDK 41.

Feel free to close this PR and proceed with the changes as you wish, or I can try to work on this, but probably I will need some assistance in a process, which might only lengthen the process unnecessary.

No matter of your decision I'm happy to help with testing new release etc. 🙂

IjzerenHein commented 3 years ago

Released as v3.0.0: https://github.com/IjzerenHein/react-native-bundle-visualizer/releases/tag/v3.0.0

IjzerenHein commented 3 years ago

Let me know whether this fixes the problem for you :D

Simek commented 3 years ago

@IjzerenHein Will test that soon, thank you so much resolving this issue and publishing new version so quickly! 🙂

I will open new issue in the future if there will be any errors or problem.

IjzerenHein commented 3 years ago

You're welcome @Simek, let me know in case you run into any problems!