search

digidem / comapeo-mobile

The next version of Mapeo mobile
GNU General Public License v3.0
5 stars 1 forks source link

fix: Fix metro config hierarchical lookup option #367

Closed gmaclennan closed 3 months ago

gmaclennan commented 3 months ago

This PR removes the custom metro config option disableHierarchicalLookup: true option. This option is problematic because it appears to break correct package resolution according to semver. In normal behaviour, if two different dependencies have the same sub-dependency, but incompatible semver versions, then the dependency will be saved in nested node_modules, to ensure each dependency gets the sub-dependency that they are expecting. Disabling this behaviour seems to cause dependencies to be incorrectly resolved, in this case for the package serialize-error. metro depends on serialize-error@2.1.0 and rpc-reflector depends on serialize-error@8.1.0. With disableHierarchicalLookup: true, rpc-reflector tries to use the version of serialize-error installed in the top-level node_modules folder, which is version @2.1.0. This causes errors in rpc-reflector because the API is different across major versions of this package.

I believe that this option will result in subtle bugs down the line in other modules. It's going to be rare that things actually crash, because in general two versions of a module are compatible, but behaviour might be different and dependencies might not be tested on these different versions.

I initially got build errors upon making this change, but I think they might have been due to the debug version of the app installed on my phone. The error I got was Error: Cannot find native module 'ExpoSplashScreen'. I fixed this with these steps:

  1. Uninstalled the debug build of my app from my development phone.
  2. Ran npx expo prebuild --clean
  3. Rebuilt and installed the app via npm run android

After these steps the app was working without error for me.

achou11 commented 3 months ago

Just adding that this should fix #364 too