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

Upgrade electron to 1.6.8 #1848

Merged
merged 6 commits into from
May 20, 2017
Merged

Upgrade electron to 1.6.8 #1848

merged 6 commits into from
May 20, 2017

Conversation

albinekb
Copy link
Contributor

Looked at #1738 etc, turns out the eval problem was easy to work around (it was because of cheap-eval-source-map), with this change source-maps still work 👌 🙌

Latest electron uses node 7.4, so we still need the --harmony-async-await (right? 🤔 ) for plugin compatibility.

Tested on macOS and seems to work as before.
need to check with windows...

This should give some nice memory optimizations etc. 💯

@albinekb albinekb assigned chabou and unassigned chabou May 20, 2017
@albinekb albinekb requested review from rauchg, chabou and matheuss May 20, 2017 01:57
@albinekb albinekb changed the title Upgrade electron Upgrade electron to 1.6.8 May 20, 2017
@@ -7,7 +7,7 @@ const nodeEnv = process.env.NODE_ENV || 'development';
const isProd = nodeEnv === 'production';

module.exports = {
devtool: isProd ? 'hidden-source-map' : 'cheap-eval-source-map',
devtool: isProd ? 'hidden-source-map' : 'inline-source-map',
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe better to use cheap-module-source-map regarding to issue in webpack that you linked in this Electron issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do

@albinekb
Copy link
Contributor Author

Done @chabou 🎉

Copy link
Contributor

@chabou chabou left a comment

Choose a reason for hiding this comment

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

Can you commit your yarn.lock? You didn't commit it after pining dependencies.

@chabou
Copy link
Contributor

chabou commented May 20, 2017

You're right! I can't reproduce #1738 anymore with this source-map format change.
It is not intellectually satisfying to accept this "regression" about eval but it is worth it! 👍

@rauchg
Copy link
Member

rauchg commented May 20, 2017

This is awesome work!

@albinekb
Copy link
Contributor Author

@chabou 🤦‍♂️ thanks for noticing 😂
❤️ @rauchg

@albinekb
Copy link
Contributor Author

Let's merge when the tests are done 😎

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

Successfully merging this pull request may close these issues.

3 participants