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

replace spread operator #87

Closed
mgrundkoetter opened this issue Oct 4, 2019 · 2 comments
Closed

replace spread operator #87

mgrundkoetter opened this issue Oct 4, 2019 · 2 comments

Comments

@mgrundkoetter
Copy link

As you can see in your forks (MarkForged@6500a64) the spread operator is not a good idea there (yet). At first, the whole thing is not backward compatible, on the other hand it is crashing even in newer versions when webpack is used.
For instance when using frameworks like vue or react inside electron, using your electron-store will already crash with the line const Store = require('electron-store'); as the spread operator will not be recognized correctly (Unexpected token ...)
See electron-userland/electron-webpack#65

@sindresorhus
Copy link
Owner

Both Babel and Webpack support object spread and have for a long time. The fact that electron-webpack uses an outdated plugin/preset is not really something this package should concern itself with. I use syntax that works in the Electron version I target. If you use outdated tools, then it's up to you to work around it, sorry.

@mgrundkoetter
Copy link
Author

Ok, maybe the provided links are a little confusing. I use the most recent versions of electron (6.0.11), react (16.9.0) and everything else, so nothing outdated at all. Also, it's not about the spread operator in general, but just the way it is used here (merging options). If you set up a new electron app with react or vue (everything most recent versions ;-), your module will crash as described just be requiring it. If you do not desire to be compatible to this combination of tools, everything is fine. I indeed found a workaround for it: just use another module (electron-settings) instead of this one.

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

No branches or pull requests

2 participants