electron-forge import #20

MarshallOfSound · 2016-12-14T11:00:32Z

This adds a command that (at least in my testing) semi-automates the transition from other build tooling (gulp, babel, -builder, Etc.) to electron-forge. Currently it:

Asks to change the main pointer of the package.json file
Imports existing babel config to the .compilerc
Warns about the slight change in usage of the preload property
Adds the electron-forge output directory to the gitignore
Adds the template forge config to their package.json
Removes electron and electron-prebuilt and replaces them with electron-prebuilt-compile
Installs dependencies like electron-compile
Prunes no longer required modules

It does all this while warning the user that we will be writing to their files and in certain steps asking if we are doing the right thing.

This command isn't complete yet (I probably want import other configs like LESS and such if they exist) and I need to do some testing. But as an example when I ran this against my rather complex GPMDP and then ran electron-forge start it launched correctly 👍

Just looking for initial feedback and maybe some enhancements to the import process.

/cc @paulcbetts because you seemed quite keen for this 😆

TODO

Should prompt (and automate) replacement of electron with electron-prebuild-compile
Should prompt (and automate) the installation of electron-compile as dependency
~~Should prompt (and automate) the installation of linting tools if none are present~~ (no longer doing this because of Support for custom template directories #35)
Should prompt the user to remove existing build tooling (babel, Electron Packager, etc.)

malept · 2016-12-14T14:50:21Z

src/electron-forge-import.js

+    const pruneSpinner = ora.ora('Pruning deleted modules').start();
+    await new Promise((resolve) => {
+      d('attempting to prune node_modules in:', dir);
+      const child = yarnOrNPMSpawn(hasYarn() ? [] : ['prune'], {


Ugh. I was hoping to avoid command specific behavior when implementing electron/packager#518 but I guess I'll have to do it.

malept · 2016-12-14T14:52:18Z

src/electron-forge-import.js

+
+    await pify(rimraf)(path.resolve(dir, 'node_modules/.bin/electron'));
+    await pify(rimraf)(path.resolve(dir, 'node_modules/.bin/electron.cmd'));
+    await pify(rimraf)(path.resolve(dir, 'node_modules', electronName));


It occurs to me that we can replace rimraf calls with fs.remove since fs-promise uses fs-extra. But that can be for another PR.

malept · 2016-12-14T14:54:49Z

src/electron-forge-import.js

+  const { shouldChangeMain } = await inquirer.createPromptModule()({
+    type: 'confirm',
+    name: 'shouldChangeMain',
+    message: 'Do you want us to change the "main" attribute of your package.json?  If you are currently using babel and pointint to a "build" directory say yes.', // eslint-disable-line


malept · 2016-12-14T14:57:28Z

src/electron-forge-import.js

+  ' "electron-prebuilt-compile" '.cyan +
+  'dependency.  This is the dependency you must version bump to get newer versions of Electron.\n\n' +
+  'We also tried to import any build tooling you already had but we can\'t get everything.  You might need to convert any CLI/gulp/grunt tasks yourself.\n\n' +
+  'Thanks for using ' + 'electron-forge'.green + '!!!');


I feel like this should be a multi-line templated string (if that works). Not a big fan of the +s and backslashes.

malept · 2016-12-14T14:58:21Z

src/electron-forge-start.js

@@ -24,7 +24,7 @@ const main = async () => {
        dir = path.resolve(dir, cwd);
      }
    })
-    .parse(process.argv);
+    .parse(process.argv.slice(0, 2));


Only parse the first two args (electron-forge and start, all the rest should be ignored or the CLI fails 😢

Ah. I completely missed the part below this where you take the rest of the arguments and pass them directly to electron.

malept · 2016-12-14T14:59:38Z

src/init/init-npm.js

      packageJSON.scripts.lint = 'eslint src';
      break;
+    default:
+      packageJSON.scripts.lint = 'echo "No linting yet..."';


"No linting configured"?

malept · 2016-12-16T15:58:33Z

Is the last thing that's missing "remove Electron Packager and other existing build tools from (dev)Dependencies"?

MarshallOfSound · 2016-12-17T05:38:55Z

@malept Pretty much yes.

Currently blocked though by electron-compile's usage of debug and debug not working well in a preload script. Upstream PR can be found here

debug-js/debug#377

anaisbetts · 2016-12-18T20:09:21Z

@MarshallOfSound I fixed this in the latest version

MarshallOfSound · 2016-12-27T03:24:54Z

/cc @malept There are a few bonus features in this PR now that seemed to be required for some projects to move over to electron-forge completely. In particular the --targets argument to the make command.

malept · 2016-12-28T01:00:28Z

src/electron-forge-package.js

+      const bins = await pify(glob)(path.join(buildPath, '**/.bin/**/*'));
+      for (const bin of bins) {
+        await fs.remove(bin);
+      }


I'm probably missing something, but can we use --no-bin-links when yarn install is run?

What about during development or if users run yarn manually?

Hmmm. Good point. For some reason I thought that Electron Packager ran npm install but it was just for its tests.

malept · 2016-12-28T06:10:03Z

I added automatic removal of all known Electron build tools from package.json, but I guess we should probably list them and confirm with the user before doing so?

MarshallOfSound · 2016-12-28T06:33:03Z

@malept Probably should prompt for each removal to be sure 👍, maybe with an brief explanation as to how -forge replaces it.

Use lodash templating to allow the config object to reuse values from the existing package.json file

…ng packaging

Preferences

MarshallOfSound requested a review from malept December 14, 2016 11:00

MarshallOfSound changed the title ~~Import~~ electron-forge import Dec 14, 2016

MarshallOfSound added the enhancement label Dec 14, 2016

malept requested changes Dec 14, 2016

View reviewed changes

MarshallOfSound force-pushed the import branch from 891c404 to 7b77621 Compare December 16, 2016 10:52

malept added the blocked/upstream Issues blocked by upstream bugs label Dec 17, 2016

MarshallOfSound force-pushed the import branch from 7b77621 to 7fbf837 Compare December 27, 2016 03:19

malept reviewed Dec 28, 2016

View reviewed changes

MarshallOfSound removed the blocked/upstream Issues blocked by upstream bugs label Dec 28, 2016

malept mentioned this pull request Dec 28, 2016

Request: electron-rebuild plugin to use with afterCopy electron/packager#359

Closed

malept force-pushed the import branch from 0f88c78 to 5a5acdb Compare December 30, 2016 02:16

malept approved these changes Dec 30, 2016

View reviewed changes

MarshallOfSound and others added 12 commits December 29, 2016 19:02

feat(importer): create inital import logic

58d320b

feat(importer): fix the projects gitignore on import

1a1ed59

chore(generic): add importer to the git-cz list

b5646ba

feat(importer): move babel config in existing project to .compilerc

2e742d5

fix(importer): update the logging as per PR feedback

d6f32d1

feat(generic): allow config options to use string templating

5c4045f

Use lodash templating to allow the config object to reuse values from the existing package.json file

fix(packager): remove stray .bin files leftover by yarn installs duri…

c8c1b00

…ng packaging

fix(rebuild): skip dependencies without a package.json file

ff311e4

fix(importer): pretty print the compilerc file

aea3271

feat(maker): allow user to override make targets

57777b5

feat(importer): delete existing Electron build tools from package.json

e483308

refactor(importer): de-rimrafify

cef743b

malept added 3 commits December 29, 2016 19:02

feat(importer): confirm build tool package removal from user

2795e29

refactor(importer): use readPackageJSON

7b1405c

Miscellaneous fixes

6f195f2

malept force-pushed the import branch from b05ed9d to 6f195f2 Compare December 30, 2016 03:03

malept merged commit 2cc94aa into master Dec 30, 2016

malept deleted the import branch December 30, 2016 03:50

dsanders11 pushed a commit that referenced this pull request Jan 14, 2023

Merge pull request #20 from electron/settings

ccdd8d7

Preferences

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

electron-forge import #20

electron-forge import #20

MarshallOfSound commented Dec 14, 2016 •

edited by malept

Loading

malept Dec 14, 2016

malept Dec 14, 2016

malept Dec 14, 2016

malept Dec 14, 2016

malept Dec 14, 2016

MarshallOfSound Dec 16, 2016

malept Dec 16, 2016

malept Dec 14, 2016

malept commented Dec 16, 2016

MarshallOfSound commented Dec 17, 2016

anaisbetts commented Dec 18, 2016

MarshallOfSound commented Dec 27, 2016

malept Dec 28, 2016

MarshallOfSound Dec 28, 2016

malept Dec 28, 2016

malept commented Dec 28, 2016

MarshallOfSound commented Dec 28, 2016

electron-forge import #20

electron-forge import #20

Conversation

MarshallOfSound commented Dec 14, 2016 • edited by malept Loading

TODO

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malept commented Dec 16, 2016

MarshallOfSound commented Dec 17, 2016

anaisbetts commented Dec 18, 2016

MarshallOfSound commented Dec 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malept commented Dec 28, 2016

MarshallOfSound commented Dec 28, 2016

MarshallOfSound commented Dec 14, 2016 •

edited by malept

Loading