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

Node modules are not installed when using Yarn workspaces #774

Closed
3 tasks done
martijnthe opened this issue Dec 11, 2017 · 10 comments
Closed
3 tasks done

Node modules are not installed when using Yarn workspaces #774

martijnthe opened this issue Dec 11, 2017 · 10 comments

Comments

@martijnthe
Copy link

martijnthe commented Dec 11, 2017

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Please describe your issue:

It seems like electron-packager doesn't play well with npm packages that have been hoisted to a node_modules parent folder. Is that by design? We're using yarn workspaces and npm packages get hoisted automatically to the root of the mono repo. Running electron "from source" works fine, because node's module resolution also looks in node_modules folder of parent folders.

Would you be interested in accepting a patch to solve this issue? Perhaps a new option key could be introduced for this?

Related issues:

Console output when you run electron-packager with the environment variable DEBUG=electron-packager. Please include the stack trace if one exists.

$ electron-packager .
Packaging app for platform darwin x64 using electron v1.7.9
Wrote new app to [redacted]/electron-quick-start/packages/electron-app/electron-quick-start-darwin-x64

What command line arguments are you passing? Alternatively, if you are using the API, what
parameters are you passing to the packager() function?

electron-packager .

Please provide either a failing minimal testcase (with a link to the code) or detailed steps to
reproduce your problem. Using electron-quick-start
is a good starting point.

https://github.com/martijnthe/electron-quick-start/tree/repro-electron-packager-issue-774

To repro:

$ yarn -v  # make sure you've a recent version of yarn that supports workspaces
1.3.2
$ cd electron-quick-start
$ yarn
$ cd packages/electron-app
$ yarn electron . # Observe the app works fine when run "from source"!
$ yarn electron-packager .
yarn run v1.3.2
$ /Users/mthe/electron-quick-start/packages/electron-app/node_modules/.bin/electron-packager .
Packaging app for platform darwin x64 using electron v1.7.9
Wrote new app to /Users/mthe/electron-quick-start/packages/electron-app/electron-quick-start-darwin-x64
✨  Done in 2.32s.

# Now open the produced app. It will immediately throw an error
# that the "is-number" package cannot be found. This package is
# an npm dependency. Yarn automatically hoists this package to
# the root of the mono repo, but the packager doesn't traverse
# parent `node_modules` folders like node does.
@welcome
Copy link

welcome bot commented Dec 11, 2017

👋 Thanks for opening your first issue here! If you have a question about using Electron Packager, read the support docs. If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. Development and issue triage is community-driven, so please be patient and we will get back to you as soon as we can.

To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@malept
Copy link
Member

malept commented Dec 11, 2017

In order to debug your problem further, we need a minimal testcase to reproduce your problem. Using the electron-quick-start repository as a base, could you please create a minimal Electron app that illustrates the issue you described, and post a link to it here?

martijnthe pushed a commit to martijnthe/electron-quick-start that referenced this issue Dec 12, 2017
@martijnthe
Copy link
Author

@malept done!

@dancinglone
Copy link

i came across with the same issue V_V

@malept
Copy link
Member

malept commented Jan 2, 2018

@martijnthe what sort of changes are you proposing?

I wonder if the best way to fix this is to have an Electron Packager plugin that runs yarn install --production in an afterCopy hook, so that node_modules is populated in the app. I'm hesitant to have the functionality I propose in the core app because it's specific to yarn, and I've already had bad experiences trying to accommodate multiple package managers.

@martijnthe
Copy link
Author

@malept that could work, will try. Thanks.

@brtthomp
Copy link

I am having a similar issue but without yarn.

In my instance I am using stylus and pug (which are css and html pre-processors). I use gulp to build each of these into a dist folder as well as copy the package.json file into the dist as well. The reason I do this is because there are different builds that need to be done. When I run electron-packager and point it to dist it will build without the node_modules folder thus causing problems.

My proposal would be that electron-packager looks up in parent directories to find a node_modules folder if one does not exist. Otherwise I am force to either run npm install or copy the modules folder over on each build.

@malept
Copy link
Member

malept commented Jan 15, 2018

@brtthomp you have a different use case than the issue summary.

Otherwise I am force to either run npm install or copy the modules folder over on each build.

Yes, those are your two options, given the current state of your build system.

I think that traversing parent directories to find a node_modules folder would add a lot of complexity to Electron Packager for the benefit of a very small percentage of users.

@malept malept changed the title electron-packager does not include packages that are hoisted to a parent node_modules folder Node modules are not installed when using Yarn workspaces Jan 15, 2018
@brtthomp
Copy link

@malept
I must have misunderstood the summary and read too much into the original title. Sorry about that.

Thanks for the update.

@malept
Copy link
Member

malept commented Jan 26, 2018

I'm going to close this since I described a workaround in #774 (comment).

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

No branches or pull requests

4 participants