-
-
Notifications
You must be signed in to change notification settings - Fork 486
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
Add file extensions to imports #294
Conversation
Add ES6 style export to utils and change related utils imports Update eslint rule for import extensions
Pull Request Test Coverage Report for Build 453
💛 - Coveralls |
getFirstMatch, | ||
getSecondMatch, | ||
} from './utils'; | ||
import Utils from './utils.js'; |
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.
Sorry, but I don't understand, why we need to replace extracted getFirstMatch
and getSecondMatch
methods to the Utils
. I thought all we needed is the ES2015 module export changes in utils.js
and the importing part would have a .js
added in the end. Do I missing something?
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.
Totally agree this isn't ideal - it happens is because Utils is currently a class with static methods.
With 'pure' es6 you can't import methods separately from a class (I didn't even realise it was possible with webpack until I saw it here :) )
The ideal option - which I mentioned in the original post above - would be to update the utils.js file into separate exports rather than static methods on a class. I'll update the PR later so you can have a look and see if it's what you want (should be back to looking how it was before except internally utils will export each function rather than export one class)
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.
Oh, right. My mistake, sorry. I've forgotten about the class. Actually, having one export produces less code in the end, because after all each module has it's own wrapper when babel transpiles it to ES5. So, no worries then and we can use it like that.
In order to support loading Bowser directly in the browser without transpilation this PR :
Unit tests appear to need no changes as functionality is the same.
Some thoughts
As utils.js is a class collection of static functions it might be better the split them into a set of separate exports. This would mean imports could look like they are in the current master branch:
rather than importing the whole of Utils.