Update to electron-forge 6 & simplify build pipeline/project structure #418
Conversation
Forgot to commit yarn.lock when removing conventional-changelog-eslint
This commit significantly simplifies the build pipeline. The lynch pin was getting rid of the resources folders, since those were what created a difference between the Electron app as it existed in the source tree, and as it existed when we invoked electron-forge. Removing that allowed us to put a proper electron-forge project inside the Ember app, complete with its own package.json and node_modules. Then the ember electron commands just need to build the ember app, put the results in the Electron app, and we have a complete electron app exactly as electron-forge would expect. This also allows us to keep the Electron dependencies separate from the Ember dependencies, which makes dependency management/maintenance much simpler.
bendemboski
force-pushed
the
ef6
branch
2 times, most recently
from
d326e4e to
d492dd9
Compare
Switch for the home-grown reload logic to using Ember's live reload server, which works more reliably and also supports hot reload of CSS
If a file/directory exists at the same path where we want to create the electron forge project when running the blueprint, error gracefully. If we see that the `ember-electron directory exists, print out a message about upgrading from v2.
Instead of overwriting .travis.yml with a blueprint file (which whacks any customizations the user has made, and requires us to keep our blueprint .travis.yml in sync with the default version as well as worrying about yarn vs npm), use js-yaml to inject the xvfb bits.
Use the blueprint to update .eslintignore to exclude the electron project's node_modules and our various built output directories
`ember electron:make` and `ember electron:package` are much more similar to `ember electron:deploy` (which defaults to building for production) than they are to `ember electron:build` (which defaults to building for development). So this switches the default environment for those commands to `production`. Making or packaging is much more similar to
We want better manual control of releases, especially while updating from 2.x to 3.x
Get us on the same node version (10) as Travis, and improve the npm caching slightly
nodejs/node#9500 was causing the hidepassed query param to not work because of the URL parsing we do in the test runner. So rather than working around it by changing to hidepassed=1 in testem-electron.js, let's fix it directly in test-runner.js.
It's apparently not needed anymore since we aren't going though all the hacky Electron wrapper scripts and stuff with electron-forge v6
Replace it with Node's built in promisify
I realized that since we support down to Node 8 and there are a number of relevant differences between it and Node 10 (e.g. promise-based filesystem APIs), we should test against Node 8 so we don't accidentally break compatibility.
The electron:test and electron:build commands are sub-classes of the built-in test and build commands, so let's build our options off of their options rather than just duplicating them all.
|
|
||
| `ember-electron` 2.x managed the `resources-*` folders to allow you to specify platform-specific resources. `electron-forge` does not support this functionality, and it was one of the main factors contributing to the complexity and slowness of `ember-electron` 2.x's build pipeline, so it's been removed in 3.x. | ||
|
|
||
| So if you only have content in `ember-electron/resources/`, you can just copy the folder into `electron-app`. Note that `src/index.js` is now in a subfolder, unlike `main.js` was in 2.x, so if you are accessing resources from your main process using, e.g., `path.join(__dirname, 'resources')`, you'll have to update it to `path.join(__dirname, '..', 'resources')` (or you could put the `resources` folder in the `src/` directory if you're some kind of monster). |
There was a problem hiding this comment.
| So if you only have content in `ember-electron/resources/`, you can just copy the folder into `electron-app`. Note that `src/index.js` is now in a subfolder, unlike `main.js` was in 2.x, so if you are accessing resources from your main process using, e.g., `path.join(__dirname, 'resources')`, you'll have to update it to `path.join(__dirname, '..', 'resources')` (or you could put the `resources` folder in the `src/` directory if you're some kind of monster). | |
| So if you only have content in `ember-electron/resources/`, you can copy the folder into `electron-app`. Note that `src/index.js` is now in a subfolder, unlike `main.js` was in 2.x, so if you are accessing resources from your main process using, e.g., `path.join(__dirname, 'resources')`, you'll have to update it to `path.join(__dirname, '..', 'resources')`. Alternatively, you could put the `resources` folder in the `src/` directory, but it is not recommended. |
There was a problem hiding this comment.
The language here is trying to emphasize that this is all you need to do as far as copying files if you don't have any platform-specific resources, but if you do have platform-specific resources, you need to do some other stuff.
Maybe
So if you only have content in
ember-electron/resources/, you can copy the folder intoelectron-appand ignore the otherember-electron/resources-*folders.
?
|
|
||
| ## Developing main process code | ||
|
|
||
| `ember-electron` does not restart your Electron app when main process code changes, so after making a code change you need to quit and re-launch your application. If you just run `ember electron` each time and you aren't changing any Ember code, you'll have to unnecessarily wait through the Ember build process that `ember electron` automatically runs. To avoid this, you can |
There was a problem hiding this comment.
| `ember-electron` does not restart your Electron app when main process code changes, so after making a code change you need to quit and re-launch your application. If you just run `ember electron` each time and you aren't changing any Ember code, you'll have to unnecessarily wait through the Ember build process that `ember electron` automatically runs. To avoid this, you can | |
| `ember-electron` does not restart your Electron app when main process code changes, so after making a code change you need to quit and re-launch your application. If you only run `ember electron` each time and you aren't changing any Ember code, you'll have to unnecessarily wait through the Ember build process that `ember electron` automatically runs. To avoid this, you can |
There was a problem hiding this comment.
Maybe it's stylistic, but this actually reads less clear to me. What's your argument against just in this context?
There was a problem hiding this comment.
I feel like only === just in meaning here, as the thing you are trying to convey is you only run ember electron.
|
|
||
| Your `electron-app` project will initially have the most recent release of Electron installed. You were probably running an older version since it was very difficult to run newer than Electron 4.x with `ember-electron` 2.x. So if you don't want to deal with upgrading Electron in the middle of upgrading `ember-electron`, you should change your `electron-app/package.json`'s `electron` version to match the version of `electron-prebuilt-compile` (which would determine the Electron version you were running) that is in your root `package.json`. Don't forget to run `yarn`/`npm install` from `electron-app` afterwards! | ||
|
|
||
| The good news is that now upgrading Electron just involves updating the version installed in the `electron-app` package -- no more waiting for `electron-prebuilt-compile` to release an update matching the latest Electron version! |
There was a problem hiding this comment.
| The good news is that now upgrading Electron just involves updating the version installed in the `electron-app` package -- no more waiting for `electron-prebuilt-compile` to release an update matching the latest Electron version! | |
| The good news is that now upgrading Electron only involves updating the version installed in the `electron-app` package -- no more waiting for `electron-prebuilt-compile` to release an update matching the latest Electron version! |
There was a problem hiding this comment.
(same comment here as above)
There was a problem hiding this comment.
I'm not the one who started the war on "just", I know it gets flagged by a lot of tools as being discriminatory or something though.
There was a problem hiding this comment.
Like is it an ESL thing? Can you link to any articles or anything?
To be blunt, I'm not inclined to follow advice that seems wrong to me if I don't have any understanding of the reasoning behind it. I'm certainly not going to die on this hill if you feel really strongly about it, but even after a little googling, I'm unable to get any sense of why only would be preferable to just here.
There was a problem hiding this comment.
I think the argument is it has a negative connotation like "just do it, it's easy", "just make it work" etc. This case doesn't necessarily apply. I don't particularly care that much, only following what I have heard. There was a talk at EmberConf about it, and it sounds like whatever tool they use only looks for how many times you said "just", "guys", etc anything that could be referring to gender or implying that something is easy, when it is not easy for everyone.
After hearing so much about how it's hard for outsiders to get into the tech industry (e.g. from Ellen Ullman's story) and considering that those whose native language is not English may not be familiar with the expression, I think you're probably right. Nevertheless, the disappearance of this kind of innocent lightheartedness (see also babel/babel@033681a) still strikes me as unfortunate. |
Guilt by association: you edited something nearby so now you have to fix it 🤣 |
🤷♂ okay. This doesn't strike me as a case that's going to really cause harm, and I believe there are soft benefits in keeping some levity and humor in our community environments, but don't feel strongly, so I'll acquiesce to the 2 against 1 vote! Unless y'all would be happy adding an emoji or something to clarify that it's a joke? 😝 Otherwise, how about:
|
There was a problem hiding this comment.
Otherwise, how about:
or you could even put the
resourcesfolder in thesrc/directory if you like, although many folks prefer to not mix their code with non-code resources
This sounds fine to me 👍 . We need to strive to be as welcoming and inclusive as we can. I do agree that it sucks we cannot have lighthearted things anymore, but it only takes one tweet of someone who did have their resources in src saying "ember.js says I'm a monster" to cause an uproar.
|
Okay. Maybe I have more faith in "the community" and maybe that's super naive, but also I don't care that much, so I'll change it. |
|
I think I've addressed all of the comments on this PR -- let me know if I've missed any. Before merging I want to port my big complex @jacobq @rwwagner90 anything you can think of that we should wait on before merging? |
|
@bendemboski I think this looks good to me now. I'm running it in my app, and it all works! We may want to document ways to deal with the two yarn installs and two package.jsons. |
I won't have a chance to migrate my main app to it until late next week, but I don't think we should hold up merging just for that. Perhaps it could merge and be a beta (3.0.0-beta.1?) release so there's a chance to make more breaking changes if major problems come to light afterward? |
|
Oh yeah, we're going to beta it for sure. I just want to do my migration before even doing that unless I get hung up and can't have it done by Monday. But yeah, definitely beta! |
Rather than trying to cobble together a decent story for linting an Electron project inside an Ember project, when the Electron tooling doesn't even really have a linting story, let's just disable it in the Electron project. electron-forge by default doesn't configure the project to lint, so let's leave it to users to do their selves and/or for some future time when we have more real-world feedback and/or electron-forge has a linting story. Make the blueprint also update .travis.yml to install dependencies in the Electron project along with Ember dependencies. Stop customizing the output directory, and let it stay where electron-forge wants to put it by default. This leaves us more in line with the default tooling, which may make things easier down the road. Also, configure the Electron app's defualt package.json to tell electron-packager to ignore our test and test build directories, so packaged apps don't end up bloated with extra stuff.
|
In case you rebase again, you may want to fix the typo in the commit message: |
|
Looks like the Windows build failures are mostly/all due to hard-coded POSIX-style path separator. These should be easily fixable by replacing things like |
The way to think about it that's been helpful to me is to realize that context is not static. What may be totally appropriate, clear, acceptable, etc. in the environment where it originated may later be terribly misconstrued. When I worked at 3M we were often encouraged to mentally run every electronic message we authored through the filter of "If you wouldn't want to see this in the headlines of a newspaper [or defend it in court] then don't write it." and there's definitely some wisdom in that. The emberjs developer community of today may later expand/change, etc. meanwhile history as recorded by version control systems doesn't have the flexibility to clarify or explain itself. |
Oops, I missed one in the previous commit. If rebasing, please squash this :)
|
I think we're ready to merge this and get a beta out. Any objections? Once this is merged, I think we should try to add https://github.com/rwjblue/create-rwjblue-release-it-setup to setup releases and changelogs for us. |
|
Sure, go for it! |
Fine by me. I thought Felix had set up something like that already though (semantic-release bot).... is that no longer working / now obsolete? |
|
@jacobq we removed it because we didn't like it releasing for us. We wanted to be able to decide when to bump versions and release, especially with releasing betas like this. |
This is a very significant, and breaking, change, but should yield very big improvements in developer ergonomics. The documentation explains in detail, but the two major changes are updating to
electron-forge6, and significantly simplifying the build pipeline:electron-forge6 allows us to use much more modern Electron tooling, including updating Electron directly rather than relying on libraries likeelectron-prebuilt-compileto releaseelectron-forgeproject, optionally allowing the direct use of all of its tooling (includingelectron-forge publish).ember electronuses the same live reload mechanism asember serve, including hot reloading CSS