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

Add ES modules for modern bundlers #20

Merged
merged 2 commits into from
Jan 15, 2018
Merged

Conversation

greggb
Copy link
Contributor

@greggb greggb commented Jan 15, 2018

This is a fantastic library. The small size and single purpose were exactly what I was looking for.

I ran into a breaking bug when building for production with webpack 3, as uglify has trouble with mixed import and module.exports (expecting export default).

I rewrote the rollup config to use a newer syntax and added an ES module export. That export was also added to .gitignore and package.json under the module property so it can be resolved by webpack.

The tests all passed and the minified browser bundle does not return the export, just a function now.

@codecov
Copy link

codecov bot commented Jan 15, 2018

Codecov Report

Merging #20 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage    99.2%   99.21%   +<.01%     
==========================================
  Files           5        5              
  Lines         126      127       +1     
  Branches       29       29              
==========================================
+ Hits          125      126       +1     
  Misses          1        1
Impacted Files Coverage Δ
src/browser.js 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e277d5...50f8d65. Read the comment docs.

Copy link
Owner

@pveyes pveyes left a comment

Choose a reason for hiding this comment

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

Nice!

I didn't experience any error when using webpack 3 but maybe because I haven't used some plugin like ModuleConcatenationPlugin

I'm assuming module field in package.json will only be used by webpack, and nodejs, so it's safe to build browser bundle

LGTM

@pveyes pveyes merged commit 94acbd5 into pveyes:master Jan 15, 2018
@greggb
Copy link
Contributor Author

greggb commented Jan 15, 2018

Thanks! My hunch is it was the mangle settings I was using, but I wasn't able to narrow it down.

I'm assuming module field in package.json will only be used by webpack

That's my impression. Only module aware bundlers (and newer node) would make use of it.

For posterity here was the error I was getting when loading the page: Cannot set property 'exports' of undefined

and in webpack logs:

WARNING in <FILEPATH WHERE HTMR IS IMPORTED>
60:11-18 "export 'default' (imported as 'convert') was not found in 'htmr'

ERROR in <BUNDLE WHERE HTMR IS IMPORTED> from UglifyJs
Invalid assignment [./node_modules/htmr/lib/htmr.min.js:7,0] <BUNDLE WHERE HTMR IS IMPORTED> 
error Command failed with exit code 2.

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.

2 participants