Skip to content
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

Support Yarn monorepo context #69

Closed
wants to merge 1 commit into from
Closed

Support Yarn monorepo context #69

wants to merge 1 commit into from

Conversation

Braden1996
Copy link
Collaborator

With Yarn, the React Native binary is hoisted from the individual workspace to the repository root.
Hence this command fails in such an environment, as the binary isn't located in the fixed path described here.

The proposed change resolves the React Native binary's location via Node mechanisms.

With Yarn, the React Native binary is hoisted from the individual workspace to the repository root.
This means the binary isn't located in the fixed path here. This change resolves the React Native binary's location via Node mechanisms.
@IjzerenHein
Copy link
Collaborator

Hi! Thanks for the contribution. I've tried verifying this locally but it seems to break the tests, because require.resolve searches for packages that are installed in the same location that react-native-bundle-visualizer is installed.

I've done some experimentation and came up with this solution:

function getReactNativeBin() {
  const localBin = './node_modules/.bin/react-native';
  if (fs.existsSync(localBin)) return localBin;
  try {
    const reactNativeDir = path.dirname(
      require.resolve('react-native/package.json')
    );
    return path.join(reactNativeDir, './cli.js');
  } catch (e) {
    console.error(
      chalk.red.bold(
        `React-native binary could not be located. Please report this issue with environment info to:\n`
      ),
      chalk.blue.bold(`-> ${require('../package.json').bugs}`)
    );
  }
}

...

const reactNativeBin = getReactNativeBin();

It then first searches the cwd node_modules folder, and then tries resolving using require.resolve.
Would that work for you as well?

@IjzerenHein IjzerenHein reopened this Nov 14, 2021
@Braden1996
Copy link
Collaborator Author

Hi! Thanks for the contribution. I've tried verifying this locally but it seems to break the tests, because require.resolve searches for packages that are installed in the same location that react-native-bundle-visualizer is installed.

I've done some experimentation and came up with this solution:

function getReactNativeBin() {
  const localBin = './node_modules/.bin/react-native';
  if (fs.existsSync(localBin)) return localBin;
  try {
    const reactNativeDir = path.dirname(
      require.resolve('react-native/package.json')
    );
    return path.join(reactNativeDir, './cli.js');
  } catch (e) {
    console.error(
      chalk.red.bold(
        `React-native binary could not be located. Please report this issue with environment info to:\n`
      ),
      chalk.blue.bold(`-> ${require('../package.json').bugs}`)
    );
  }
}

...

const reactNativeBin = getReactNativeBin();

It then first searches the cwd node_modules folder, and then tries resolving using require.resolve. Would that work for you as well?

Hey @IjzerenHein, I tested this locally in my repo and it seems to do the trick. Apologies for not considering the test cases with my initial PR.

@IjzerenHein
Copy link
Collaborator

Great, glad to hear that works. I'll see to it this gets merged shortly 👍

@IjzerenHein
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants