-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -67,7 +67,7 @@ module.exports = { | |||
}, | |||
{ | |||
test: /\.js$/, | |||
include: /node_modules\/(material-chip-input|ethereumjs-tx)/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this necessary? I think the code should work in any browser supporting ES6. Do we support other browsers anyway?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It shouldn't be. We are still using UglifyJS to pack the bundle, and UglifyJS only understands ES5 syntax. I think the only thing that is ES6 there are const
and let
declarations, so we could either kick those out, or change the minifier to Babili or something else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. So it's ok then, was just making sure that it's needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have something in the back of my head to look at the new Babel uglifier, thanks for the reminder in logging that - just need to see if there are any benefits/drawbacks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Analog to #5331 - use external repo with single source for the wordlist recovery phrases are generated from.