search

bugsnag / bugsnag-expo

MIT License
11 stars 5 forks source link

plugin-expo-eas-sourcemaps is not compatible with monorepos #46

Open thorbenprimke opened 2 years ago

thorbenprimke commented 2 years ago

Describe the bug

Add the plugin-expo-eas-sourcemaps config plugin to an expo project in a monorepo. It does not work due to the node_modules path not being what is expected in the scripts.

Steps to reproduce

  1. Set up a monorepo with workspaces
  2. Add the app at like packages/app
  3. Kick off an EAS build and see it fail

Environment

Example code snippet - patch packages that resolves it (but not in a dynamic manner)

diff --git a/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh b/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh
index cc52c28..bce3d79 100755
--- a/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh
+++ b/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh
@@ -58,4 +58,4 @@ if [ ! -z "$ENDPOINT" ]; then
   ARGS+=("$ENDPOINT")
 fi

-../node_modules/.bin/bugsnag-source-maps upload-react-native "${ARGS[@]}"
+../../../node_modules/.bin/bugsnag-source-maps upload-react-native "${ARGS[@]}"
diff --git a/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/src/ios.js b/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/src/ios.js
index e18e9c8..b01bd40 100644
--- a/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/src/ios.js
+++ b/node_modules/@bugsnag/plugin-expo-eas-sourcemaps/src/ios.js
@@ -31,7 +31,7 @@ function withIosPlugin (config, onPremConfig) {
     bundleReactNativePhase.shellScript = modifiedScript

     // 03. Configure the new build phase
-    const shellScript = 'SOURCE_MAP="$TMPDIR/$(md5 -qs "$CONFIGURATION_BUILD_DIR")-main.jsbundle.map" ../node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh'
+    const shellScript = 'SOURCE_MAP="$TMPDIR/$(md5 -qs "$CONFIGURATION_BUILD_DIR")-main.jsbundle.map" ../../../node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh'

     xcodeProject.addBuildPhase([], buildPhaseName, 'Upload source maps to Bugsnag', null, {
       shellPath: '/bin/sh',
gingerbenw commented 2 years ago

Hi @thorbenprimke, thanks for raising this one - I'm glad the plugin is working otherwise!

Could I confirm what your eas build command looks like, and which directory you are running it from? Are you hoisting the dependencies for your expo app?

Thanks!

thorbenprimke commented 2 years ago

Sure.

Commend: STAGE=production eas build --profile production --platform ios --auto-submit

I run this from the folder: /Users/thorben/projects/backstage-app/packages/expo. The node_modules folder is at the project root: /Users/thorben/projects/backstage-app/

I believe the answer is yes to are you hoisting the dependencies. It is set up as workspaces in the root package.json and all dependencies are installed at the root but defined in the expo folder's package.json.

Let me know if I can provide anything else or you want to me to test a change.

gingerbenw commented 2 years ago

Hi @thorbenprimke - Apologies for the delay on this, could you please try the following:

Install @bugsnag/source-maps as a dev dependency to your expo application, and add the following to the package.json (also in the expo app):

"workspaces": {
    "nohoist": [
      "@bugsnag/plugin-expo-eas-sourcemaps",
      "@bugsnag/source-maps"
    ]
  }

This will prevent the dependencies being hoisted, and allow the plugin to execute the necessary binaries from the node_modules.

evelant commented 2 years ago

@gingerbenw That won't work for people running modern yarn. nohoist only works on yarn classic (1.x versions).

gingerbenw commented 2 years ago

Hi @evelant! Apologies for the slow response, I needed to do some homework on this one.

nohoist works when using the node modules plugin which is required when using expo with yarn 2 and above.

we're looking to automate adding this nohoist option in our CLI shortly.

evelant commented 2 years ago

@gingerbenw Unfortunately it's not that simple.

IIRC when I went though this with yarn (it is confusing!) I found out that won't do it either. I'm pretty sure nohoist does not work with when using nodeLinker: node-modules. There is no way to do per-package hoisting in yarn v2+.

You first have to add nodeLinker: node-modules which turns off "plug n' play" mode to give you a node_modules folder again. Then if you add nohoist to your package.json it will not do anything as it's only supported in yarn v1. In v2+ the only option is install config hoisting limits.

Unfortunately with installConfig: { hoistingLimits: "" } the only options are none (hoist everything) dependencies (hoist everything except direct dependencies) and workspaces (don't hoist anything inside a workspace, treat it as if it were not a workspace).

I have no idea why the yarn team decided to remove per-package hoisting control. It makes things significantly more difficult.

Personally I've moved on to https://pnpm.io as it's far more consistent, fast, configurable, and reliable. Pnpm works well with react-native if you use @rnx-kit/metro-resolver-symlinks to work around metro's 5+ years unresolved lack of symlink support 🤦

gingerbenw commented 2 years ago

Thanks for the useful breakdown @evelant! I think I had misunderstood what was going on in my local testing as I had hoistingLimits: "workspace" set as well as using nohoist.

I wonder whether the best thing to do would be to symlink the necessary dependencies into node_modules in a similar fashion to the expo-yarn-workspaces plugin... 🤔

evelant commented 2 years ago

@gingerbenw I think the way a lot of packages and gradle scripts are doing it now is to use node to resolve the path, such as node --print require.resolve('react-native/package.json') although that assumes node is available on $PATH. That has the benefit of (mostly, looking at you metro) resolving files in the same way all the other tools in the stack are doing it.

gingerbenw commented 2 years ago

After some consideration we’re going to include installConfig.hoistingLimits on our CLI and documentation to get this functional for yarn 2+ in the short term and I will include making our source map upload compatible with hoisting for consideration on our roadmap.

johnkiely1 commented 2 years ago

Just to let you know we have released the short term changes mentioned above as of v45.1.2 (for Expo 45) and v46.0.1 (for Expo 46) and updated the documentation to match https://docs.bugsnag.com/platforms/react-native/expo/manual-setup/#eas-build

kennethlynne commented 1 year ago

This is still a problem for us. We're using PNPM in a monorepo with a managed expo app using SDK 49, I have not been able to get this to work yet

johnkiely1 commented 1 year ago

Hi @kennethlynne,

PNPM is not something we are particularly familiar with as we don't directly support it. As a suggestion have you tried modifying hoist-pattern or public-hoist-pattern config settings? https://pnpm.io/npmrc#hoist-pattern.

This looks to be somewhat similar to Yarn's nohoist option to avoid node_modules being hoisted to the top-level of the monorepo so that it can find the bugsnag-expo-xcode.sh script.

ricardoribas commented 1 year ago

Hey guys, I was able to make it work on the project i am working on. In order to make it work without nohoist, I had to make a few changes:

  1. Add a custom expo plugin replicating the original dependency
  2. Create a file with the following contents 
    
if [[ -f "$PODS_ROOT/../.xcode.env" ]]; then
source "$PODS_ROOT/../.xcode.env"
fi
if [[ -f "$PODS_ROOT/../.xcode.env.local" ]]; then
source "$PODS_ROOT/../.xcode.env.local"
fi

SOURCE_MAP="$TMPDIR/$(md5 -qs "$CONFIGURATION_BUILD_DIR")-main.jsbundle.map" $NODE_BINARY --print "require('path').dirname(require.resolve('@app/my-package/package.json')) + '/lib/bugsnag-expo-xcode.sh'"

3. Create a `bugsnag-expo-xcode.sh` inside the package **lib** folder. This will have the same contents as `@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh`, except the call to the bugsnag-source-maps to upload a react native, which instead will call to a local script 
```bash
$NODE_BINARY ../lib/bugsnag-expo-node upload-react-native "${ARGS[@]}"

The node executable is attached on the build phase script

  1. This new nodeJS script will not use relative paths. Instead it will: 
    
#!/usr/bin/env node

// eslint-disable-next-line @typescript-eslint/no-var-requires const { dirname, join } = require('path');

// eslint-disable-next-line @typescript-eslint/no-var-requires require(join( dirname(require.resolve('@bugsnag/source-maps/package.json')), 'dist/bin/cli.js' )).default(process.argv);



I created two PRs to address this issue under [@bugsnag/source-maps](https://github.com/bugsnag/bugsnag-source-maps/pull/87) and [@bugsnag/expo](https://github.com/bugsnag/bugsnag-expo/pull/147)
GSTJ commented 6 months ago

I'm using this patch based on the open PR, which stands stale.

It's a little long as prettier also did style changes automatically - though, it does the trick.


diff --git a/lib/bugsnag-expo-xcode.sh b/lib/bugsnag-expo-xcode.sh
index 3345711939266156933d19c58cf1c3082b5b3e06..af1ef16f56fe590129b8807a5831c033b68190ea 100755
--- a/lib/bugsnag-expo-xcode.sh
+++ b/lib/bugsnag-expo-xcode.sh
@@ -60,4 +60,11 @@ if [ ! -z "$ENDPOINT" ]; then
   ARGS+=("$ENDPOINT")
 fi

-../node_modules/.bin/bugsnag-source-maps upload-react-native "${ARGS[@]}"
+# Make sure that node binary exists
+source `$NODE_BINARY --print "require('path').dirname(require.resolve('react-native/package.json')) + '/scripts/node-binary.sh'"`
+
+# Retrieve the expo node binary
+BUGSNAG_EXPO_NODE_BINARY=$($NODE_BINARY --print "require('path').dirname(require.resolve('@bugsnag/source-maps/bin/cli.js'))")
+
+# Start upload the source maps to bugsnag
+$BUGSNAG_EXPO_NODE_BINARY upload-react-native "${ARGS[@]}"
diff --git a/src/ios.js b/src/ios.js
index 5150364c16229665014fa26d4c298558d1ef26ad..659133f8a6a8874444841cc9185956bf0df90655 100644
--- a/src/ios.js
+++ b/src/ios.js
@@ -1,54 +1,73 @@
-const {
-  withInfoPlist,
-  withXcodeProject
-} = require('@expo/config-plugins')
+const { withInfoPlist, withXcodeProject } = require("@expo/config-plugins");

-const buildPhaseName = 'PBXShellScriptBuildPhase'
-const buildPhaseComment = 'Bundle React Native code and images'
+const buildPhaseName = "PBXShellScriptBuildPhase";
+const buildPhaseComment = "Bundle React Native code and images";

-function withIosPlugin (config, onPremConfig) {
+function withIosPlugin(config, onPremConfig) {
   // 01. Update InfoPlist with apiKey
-  config = withInfoPlist(config, config => {
-    const apiKey = config?.extra?.bugsnag?.apiKey
+  config = withInfoPlist(config, (config) => {
+    const apiKey = config?.extra?.bugsnag?.apiKey;
     config.modResults.bugsnag = {
-      apiKey: apiKey
-    }
-    return config
-  })
+      apiKey: apiKey,
+    };
+    return config;
+  });

-  config = withXcodeProject(config, config => {
-    const xcodeProject = config.modResults
+  config = withXcodeProject(config, (config) => {
+    const xcodeProject = config.modResults;

     // 02. Update react native bundle phase with sourcemap filepath
-    const bundleReactNativePhase = xcodeProject.pbxItemByComment(buildPhaseComment, buildPhaseName)
+    const bundleReactNativePhase = xcodeProject.pbxItemByComment(
+      buildPhaseComment,
+      buildPhaseName
+    );

-    const initialScript = bundleReactNativePhase.shellScript
+    const initialScript = bundleReactNativePhase.shellScript;

-    const additionalExports = '"export EXTRA_PACKAGER_ARGS=\\"--sourcemap-output $TMPDIR/$(md5 -qs \\"$CONFIGURATION_BUILD_DIR\\")-main.jsbundle.map\\"\\n'
+    const additionalExports =
+      '"export EXTRA_PACKAGER_ARGS=\\"--sourcemap-output $TMPDIR/$(md5 -qs \\"$CONFIGURATION_BUILD_DIR\\")-main.jsbundle.map\\"\\n';

     if (initialScript.indexOf(additionalExports) < 0) {
-      const modifiedScript = additionalExports + initialScript.substr(1)
-      bundleReactNativePhase.shellScript = modifiedScript
+      const modifiedScript = additionalExports + initialScript.substr(1);
+      bundleReactNativePhase.shellScript = modifiedScript;
     }

     // 03. Configure the new build phase
-    const uploadBuildPhaseComment = 'Upload source maps to Bugsnag'
+    const uploadBuildPhaseComment = "Upload source maps to Bugsnag";

-    const uploadBuildPhase = xcodeProject.pbxItemByComment(uploadBuildPhaseComment, buildPhaseName)
+    const uploadBuildPhase = xcodeProject.pbxItemByComment(
+      uploadBuildPhaseComment,
+      buildPhaseName
+    );

     if (!uploadBuildPhase) {
-      const shellScript = 'SOURCE_MAP="$TMPDIR/$(md5 -qs "$CONFIGURATION_BUILD_DIR")-main.jsbundle.map" ../node_modules/@bugsnag/plugin-expo-eas-sourcemaps/lib/bugsnag-expo-xcode.sh'
+      const shellScript = `
+if [[ -f "$PODS_ROOT/../.xcode.env" ]]; then
+  source "$PODS_ROOT/../.xcode.env"
+fi
+if [[ -f "$PODS_ROOT/../.xcode.env.local" ]]; then
+  source "$PODS_ROOT/../.xcode.env.local"
+fi
+
+SOURCE_MAP="$TMPDIR/$(md5 -qs "$CONFIGURATION_BUILD_DIR")-main.jsbundle.map" \`$NODE_BINARY --print "require('path').dirname(require.resolve('@bugsnag/plugin-expo-eas-sourcemaps/package.json')) + '/lib/bugsnag-expo-xcode.sh'"\`
+      `;

-      xcodeProject.addBuildPhase([], buildPhaseName, uploadBuildPhaseComment, null, {
-        shellPath: '/bin/sh',
-        shellScript: shellScript
-      })
+      xcodeProject.addBuildPhase(
+        [],
+        buildPhaseName,
+        uploadBuildPhaseComment,
+        null,
+        {
+          shellPath: "/bin/sh",
+          shellScript: shellScript,
+        }
+      );
     }

-    return config
-  })
+    return config;
+  });

-  return config
+  return config;
 }

-module.exports = { withIosPlugin }
+module.exports = { withIosPlugin };
``