-
Notifications
You must be signed in to change notification settings - Fork 320
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
Update build tools #227
Update build tools #227
Conversation
Codecov Report
@@ Coverage Diff @@
## master #227 +/- ##
===========================================
+ Coverage 68.38% 85.29% +16.91%
===========================================
Files 5 5
Lines 136 136
===========================================
+ Hits 93 116 +23
+ Misses 43 20 -23
Continue to review full report at Codecov.
|
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.
Awesome, will merge after fixes
package.json
Outdated
"prepublish": "npm run clean && npm test && npm run build", | ||
"uglify": "uglifyjs dist/index.umd.js -cm -o dist/index.umd.min.js", | ||
"rollup": "node rollup.config.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.
Can move scripts as they appears? For example, build
run rollup
, so rollup
must be higher than build
rollup.config.js
Outdated
@@ -0,0 +1,55 @@ | |||
const rollup = require('rollup').rollup; |
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.
We don't need non-minified versions, so you can produce minified versions right here
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.
Sure. But I'm still in doubt if I should bundle two different versions, one using UMD and another CommonJS modules. Maybe just one is enough. What do you think @evgenyrodionov ?
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 think UMD is enough
Any thoughts @evgenyrodionov? |
Looks good, but I need test it. On Monday, I guess. |
I hope you don't mind @evgenyrodionov. I am taking advantage of this pull request to add some tests. |
Thank you! It looks pretty awesome. |
Thanks @evgenyrodionov!!! |
As I mentioned above if you use a module bundler like webpack or rollup, it will get the code from the source files. That can cause problems like #229 when you don't use the babel plugin to handle the object spread operator. So I think that we can either explain that on readme and ask for people use the plugin or just expose the bundled file as was made before and prevent these problems. What do you think @evgenyrodionov? |
Object spread operator already fixed in 3.0.3, it's stage 3 proposal (candidate), so it was my mistake. |
Sorry if any of the changes are out of scope.
CHANGELOG
eslintignore
.package.json
.deep-diff
.open-url
becausehttp-server
already has an option to open the browser after starting the server.babel-cli
and others unused plugins and presets.pkg.module
andpkg.jsnext:main
.