Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

WIP: #26 Move to electron-builder #236

Closed
wants to merge 13 commits into from
Closed

WIP: #26 Move to electron-builder #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ddaed5f
Have electron-builder working with webpack
Jtfinlay Sep 9, 2018
e444544
Successfully able to spawn yarn
Jtfinlay Sep 9, 2018
ab29ee2
include icon
Jtfinlay Sep 9, 2018
82a20ac
Rename to electron, and clean up
Jtfinlay Sep 9, 2018
23d5997
512x512 icon for mac
Jtfinlay Sep 9, 2018
3b44b51
sync with master
Jtfinlay Sep 9, 2018
93e25ea
Update electron asset ref
Jtfinlay Sep 10, 2018
2631c76
Have paths working correctly for assets needed from electronjs
Sep 10, 2018
66a3738
add linux to dist:all
Sep 10, 2018
ffd2a59
Include the precommit
Jtfinlay Sep 10, 2018
afdee72
wip
Jtfinlay Sep 10, 2018
e40d55a
fix image paths
Jtfinlay Sep 11, 2018
f2ef82a
remove extra indent changes
Jtfinlay Sep 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# production
/build
/release-builds
/dist

# misc
.DS_Store
Expand Down
1 change: 1 addition & 0 deletions config/paths.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ module.exports = {
appPublic: resolveApp('public'),
appHtml: resolveApp('public/index.html'),
appIndexJs: resolveApp('src/index.js'),
electronJs: resolveApp('src/electron.js'),
appPackageJson: resolveApp('package.json'),
appSrc: resolveApp('src'),
testsSetup: resolveApp('src/setupTests.js'),
Expand Down
16 changes: 12 additions & 4 deletions config/webpack.config.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,16 +118,20 @@ module.exports = {
// We generate sourcemaps in production. This is slow but gives good results.
// You can exclude the *.map files from the build during deployment.
devtool: shouldUseSourceMap ? 'source-map' : false,
// In production, we only want to load the polyfills and the app code.
entry: [require.resolve('./polyfills'), paths.appIndexJs],
// In production, we only want to load the polyfills, the app code, and the
// electron builder entry script.
entry: {
main: [require.resolve('./polyfills'), paths.appIndexJs],
electron: paths.electronJs,
},
output: {
// The build folder.
path: paths.appBuild,
// Generated JS file names (with nested folders).
// There will be one main bundle, and one file per asynchronous chunk.
// We don't currently advertise code splitting but Webpack supports it.
filename: 'static/js/[name].[chunkhash:8].js',
chunkFilename: 'static/js/[name].[chunkhash:8].chunk.js',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the hashes are added so that Chrome knows when to invalidate its cache of the JS bundles, right?

Right now that's fine since this file never changes, but presumably once we add the ability to update, we'll need to know when the file's changed.

What was the reason for this change? I suspect I'm misunderstanding something haha.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I hadn't considered whether the updater uses the hash to update changed files as well. I made this change under assumption that it wasn't needed for electron, but if it is needed then that makes this difficult.

I did this because the package.json has to reference the entry script (electron.js). Since the file is generated by webpack, it breaks the static reference if there's a dynamic chunk name as part of the file path.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh gotcha. Ok, let's leave it as-is for now. If that does become an issue, it's one we can address later on (really we should disable Chrome's browser cache altogether... and it might already be)

filename: '[name].js',
chunkFilename: '[name].chunk.js',
// We inferred the "public path" (such as / or /my-project) from homepage.
publicPath: publicPath,
// Point sourcemap entries to original disk location (format as URL on Windows)
Expand Down Expand Up @@ -370,6 +374,7 @@ module.exports = {
new HtmlWebpackPlugin({
inject: true,
template: paths.appHtml,
excludeChunks: ['electron'],
minify: {
removeComments: true,
collapseWhitespace: true,
Expand Down Expand Up @@ -484,4 +489,7 @@ module.exports = {
callback();
},
],
node: {
__dirname: false,
},
};
23 changes: 14 additions & 9 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,17 @@
"productName": "Guppy",
"version": "0.2.0",
"private": true,
"main": "src/main.js",
"main": "src/electron.js",
"homepage": "./",
"build": {
"appId": "com.electron.guppy",
"files": [
"build/**/*"
],
"asarUnpack": [
"node_modules/yarn/"
]
},
"repository": {
"type": "git",
"url": "https://github.com/joshwcomeau/guppy.git"
Expand All @@ -14,12 +23,8 @@
"start": "cross-env NODE_ENV=development npm run start:react",
"start:react": "cross-env BROWSER=none node scripts/start.js",
"build": "node scripts/build.js",
"package:linux": "electron-packager . --platform=linux --prune=true --out=release-builds --overwrite",
"package:mac": "electron-packager . --platform=darwin --arch=x64 --icon=src/assets/icons/mac/logo.icns --prune=true --out=release-builds --overwrite",
"package:win": "electron-packager . --platform=win32 --arch=x64 --icon=src/assets/icons/win/logo.ico --prune=true --out=release-builds --overwrite",
"dist:linux": "cross-env GENERATE_SOURCEMAP=false npm run build && npm run package:linux",
"dist:mac": "cross-env GENERATE_SOURCEMAP=false npm run build && npm run package:mac",
"dist:win": "cross-env GENERATE_SOURCEMAP=false npm run build && npm run package:win",
"dist": "cross-env GENERATE_SOURCEMAP=false npm run build && electron-builder -c.extraMetadata.main=build/electron.js",
"dist:all": "cross-env GENERATE_SOURCEMAP=false npm run build && electron-builder -mwl -c.extraMetadata.main=build/electron.js",
"test": "node scripts/test.js --env=node",
"coverage": "yarn test --coverage",
"report-coverage": "yarn coverage && codecov",
Expand Down Expand Up @@ -73,7 +78,7 @@
"dotenv": "5.0.0",
"dotenv-expand": "4.2.0",
"electron": "2.0.1",
"electron-packager": "12.1.0",
"electron-builder": "20.28.4",
"eslint": "4.15.0",
"eslint-config-react-app": "3.0.0-next.66cc7a90",
"eslint-loader": "1.9.0",
Expand Down Expand Up @@ -204,4 +209,4 @@
"icon": null,
"createdAt": 1529502079329
}
}
}
Binary file added public/256x256.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added public/icon.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
7 changes: 5 additions & 2 deletions src/main.js → src/electron.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ const killProcessId = require('./services/kill-process-id.service');
const electronStore = require('./services/electron-store.service');

const chalk = new chalkRaw.constructor({ level: 3 });
let icon256Path = '../public/256x256.png';

// In production, we need to use `fixPath` to let Guppy use NPM.
// For reasons unknown, the opposite is true in development; adding this breaks
// everything.
if (process.env.NODE_ENV !== 'development') {
icon256Path = '../build/256x256.png';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not thrilled by this, but I couldn't find an easy way to keep a reference to the image in both yarn run and after build that would satisfy both environments.

fixPath();
}

Expand All @@ -43,7 +45,7 @@ function createWindow() {
height: 768,
minWidth: 777,
titleBarStyle: 'hidden',
icon: path.join(__dirname, 'assets/icons/png/256x256.png'),
icon: path.join(__dirname, icon256Path),
});

// set up some chrome extensions
Expand Down Expand Up @@ -80,6 +82,7 @@ function createWindow() {
slashes: true,
});
mainWindow.loadURL(startUrl);
mainWindow.toggleDevTools();

// Emitted when the window is closed.
mainWindow.on('closed', function() {
Expand Down Expand Up @@ -184,7 +187,7 @@ const manageApplicationLocation = () => {
message: 'Move to Applications folder?',
detail:
"I see that I'm not in the Applications folder. I can move myself there if you'd like!",
icon: path.join(__dirname, 'assets/icons/png/256x256.png'),
icon: path.join(__dirname, icon256Path),
cancelId: 1,
defaultId: 0,
checkboxLabel: 'Do not show this message again',
Expand Down
2 changes: 1 addition & 1 deletion src/services/platform.service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const PACKAGE_MANAGER_CMD = path.join(
remote.app.getAppPath(),
'node_modules/yarn/bin',
formatCommandForPlatform(PACKAGE_MANAGER)
);
).replace('app.asar', 'app.asar.unpacked');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this required? Can you please give some details?
What was the error message if you're not replacing it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once unpacking yarn, it is placed under the directory \app.asar.unpacked\node_modules.. instead of within the \app.asar\node_modules.. asar file. Even though it was unpacked, code is still referencing it within the app.asar, so replacing the path.

Same issue is discussed here. I elected to workaround since this was the only case I knew of, but we could instead use the hazardous package referenced in the discussion if desired.


// Forward the host env, and append the
// project's .bin directory to PATH to allow
Expand Down
Loading