Improved yarn support #289

stopdropandrew · 2017-08-10T20:11:32Z

New project setup:
ember new ee-test --yarn
cd ee-test/
ember install ember-electron
ember electron:package

log.gauge.isEnabled is not a function
TypeError: log.gauge.isEnabled is not a function
    at Object.<anonymous> (/.../ee-test/node_modules/npm/node_modules/npmlog/log.js:57:33)
...

Initial workaround:
rm -rf node_modules
yarn upgrade

But I want electron-packager to use yarn!
sed -i '' 's/\"electronPackagerConfig\": {}/\"electronPackagerConfig\": { \"packageManager\": \"yarn\" }/' ember-electron/electron-forge-config.js
ember electron:package

...
⠧ Packaging ApplicationCommand failed: yarn install --production --no-bin-links
warning electron-prebuilt-compile > electron-compilers > [email protected]: Jade has been renamed to pug, please install the latest version of pug instead of jade
warning ember-electron > ember-inspector > ember-new-computed > ember-cli-babel > broccoli-babel-transpiler > babel-core > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning ember-electron > electron-forge > [email protected]: Use mz or fs-extra^3.0 with Promise Support
warning electron-prebuilt-compile > electron-compilers > jade > [email protected]: Deprecated, use jstransformer
warning ember-electron > electron-forge > zip-folder > archiver > glob > [email protected]: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
warning ember-electron > npmi > npm > request > [email protected]: Use uuid module instead
warning ember-electron > electron-forge > electron-windows-store > flatten-packages > [email protected]: wrench.js is deprecated! You should check out fs-extra (https://github.com/jprichardson/node-fs-extra) for any operations you were using wrench for. Thanks for all the usage over the years.
warning ember-electron > electron-forge > nugget > progress-stream > through2 > xtend > [email protected]: 
error /Users/andrew/Library/Caches/Yarn/v1/npm-electron-1.7.5-065a3102bf8b87102df50c50985fefe6c569045b: Command failed.
Exit code: 1
Command: sh
Arguments: -c node install.js
Directory: /Users/andrew/Library/Caches/Yarn/v1/npm-electron-1.7.5-065a3102bf8b87102df50c50985fefe6c569045b
Output:
module.js:442
    throw err;
    ^

Error: Cannot find module 'extract-zip'
    at Function.Module._resolveFilename (module.js:440:15)
    at Function.Module._load (module.js:388:25)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/andrew/Library/Caches/Yarn/v1/npm-electron-1.7.5-065a3102bf8b87102df50c50985fefe6c569045b/install.js:9:15)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)

Error: Command failed: yarn install --production --no-bin-links
...

There are a handful of intertwined issues here.

EE ignores shrinkwrap and yarn lock files when assembling (causes the issue in electron-packager above)
EE always uses npm to install production dependencies when assembling
checking in yarn.lock means that tests are not run against the same dependencies that users are running against

bendemboski

I very much like this! One quibble, though...

bendemboski · 2017-08-10T20:28:55Z

lib/tasks/assemble.js

@@ -1,7 +1,8 @@
 'use strict';

 const chalk = require('chalk');
-const install = require('rsvp').denodeify(require('npmi'));
+const pruner = require('rsvp').denodeify(require('electron-packager/prune').pruneModules);


I don't think this is really a public API, so I'm kinda uncomfortable using it like this. My preference would be to copy the code that we need into this project, but failing that, at the very least list electron-packager as one of our dependencies, rather than just implicitly relying on electron-forge to make it available to us.

Very fair, I felt uncomfortable doing it as well. Wanted to show intent and happy to make any changes.

bendemboski · 2017-08-10T20:32:10Z

lib/tasks/assemble.js

@@ -1,7 +1,8 @@
 'use strict';

 const chalk = require('chalk');
-const install = require('rsvp').denodeify(require('npmi'));
+const pruner = require('rsvp').denodeify(require('electron-packager/prune').pruneModules);
+const { hasYarn } = require('electron-forge/dist/util/yarn-or-npm');


I also think maybe here we should just use the yarn-or-npm package directly, rather than using a private API in electron-forge. AFAIK electron-forge only wraps it so it can introduce an environment variable-based override for its automated testing. We don't currently need that, and if we did, I'd rather have our own implementation so we don't break if electron-forge changes how they do it.

stopdropandrew · 2017-08-16T06:10:16Z

This should be good to go.

bendemboski

Looks good to me -- @felixrieseberg if you're happy with it, merge away!

felixrieseberg · 2017-08-19T00:43:54Z

Hey! I have one last comment: I want this to work with Yarn as much as possible, but I don't want yarn to be the default runner.

Electron is cool (and better than a PWA) because you can use it with native code. Native modules and Yarn still aren't friends (yarnpkg/yarn#756), so until that is the case, I'd like to stick with npm.

As it is right now, the package.json scripts require yarn, which I'm not suuuuper excited about. Could we go with npm or at least yarn-or-npm?

Remove yarn dependency Convert npm scripts to yarn Remove yarn.lock Add rejection values to workaround RSVP bug

…ncies

BenV · 2017-08-22T02:16:51Z

@felixrieseberg Is this what you had in mind?

felixrieseberg · 2017-08-25T21:18:46Z

Great, bank you!

bendemboski requested changes Aug 10, 2017

View reviewed changes

stopdropandrew force-pushed the improved-yarn-support branch 2 times, most recently from 27096e3 to b8d4dfa Compare August 16, 2017 05:58

stopdropandrew force-pushed the improved-yarn-support branch from b8d4dfa to af6d8b6 Compare August 16, 2017 15:28

bendemboski approved these changes Aug 16, 2017

View reviewed changes

stopdropandrew and others added 6 commits August 21, 2017 20:04

🔧 Improve yarn tooling

6d425d6

Remove yarn dependency Convert npm scripts to yarn Remove yarn.lock Add rejection values to workaround RSVP bug

🔧 Add npm/yarn lock files to be used during packaging

627d94e

🔧 Use electron-packager's prune modules to install production depende…

e586ecd

…ncies

🔧 Combine before_install in .travis.yml

5788d7b

🐛 Prevent watching of project root

f87d053

🔧 Use yarn-or-npm for package scripts

9d7e9b8

BenV force-pushed the improved-yarn-support branch from af6d8b6 to 9d7e9b8 Compare August 22, 2017 02:15

felixrieseberg merged commit 5c867a7 into adopted-ember-addons:master Aug 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved yarn support #289

Improved yarn support #289

stopdropandrew commented Aug 10, 2017

bendemboski left a comment

bendemboski Aug 10, 2017

stopdropandrew Aug 10, 2017

bendemboski Aug 10, 2017 •

edited

Loading

stopdropandrew commented Aug 16, 2017

bendemboski left a comment

felixrieseberg commented Aug 19, 2017

BenV commented Aug 22, 2017

felixrieseberg commented Aug 25, 2017

Improved yarn support #289

Improved yarn support #289

Conversation

stopdropandrew commented Aug 10, 2017

bendemboski left a comment

Choose a reason for hiding this comment

bendemboski Aug 10, 2017

Choose a reason for hiding this comment

stopdropandrew Aug 10, 2017

Choose a reason for hiding this comment

bendemboski Aug 10, 2017 • edited Loading

Choose a reason for hiding this comment

stopdropandrew commented Aug 16, 2017

bendemboski left a comment

Choose a reason for hiding this comment

felixrieseberg commented Aug 19, 2017

BenV commented Aug 22, 2017

felixrieseberg commented Aug 25, 2017

bendemboski Aug 10, 2017 •

edited

Loading