clipboard-copy icon indicating copy to clipboard operation
clipboard-copy copied to clipboard

Module Inclusion

Open vdegenne opened this issue 5 years ago • 7 comments
trafficstars

Hello,

I tried to use your package in an es module context only to realize it didn't work because your main index.js file possesses commonjs syntax. I decided to fork and modify the code to include module and umd support using microbundle (more info: https://github.com/developit/microbundle).

I didn't add or remove procedures, I just transformed the index.js into ES6 Module syntax which is now supported by modern browsers. There's a build step which converts this file into different targets (into dist) :

  • CommonJs (fallback) for people using plain JS in Node.JS scripts.
  • ESM (along with the module property in package.json) for people using ES6 Modules for modern browsers, experimental Node.JS module support (future), and TypeScript module resolution.
  • UMD for Node & CDN's

The test passes and the module.exports is unchanged which means existing projects won't be affected. I also tested the compilation result in my initial project and the node resolver uses the mjs as intended (i.e. @vdegenne/clipboard-copy for live testing).

I think this is one step into JavaScript evolution towards trying to make modular programming easier. And this PR should be considered.

If you merge this PR, the next step for you is to build the distribs npm run build, change the versioning, and push the update to the npm repository for everyone's sake. Also modifying the readme according to the new terms.

Have a nice day.

vdegenne avatar Dec 30 '19 13:12 vdegenne

Thank you for this PR. I would like to support ESM in this package, but I don't like how microbundle seems to do a lot more than just this. It's doing some transpiling of async-await to Promise, etc.

Is there are an tool that just takes ESM and outputs CJS and updates package.json? cc @mikeal

feross avatar Nov 18 '20 22:11 feross

Rollup would work fine in this case. Microbundle is based on it.

TrySound avatar Nov 19 '20 10:11 TrySound

Btw would you like to add node es modules support as well?

TrySound avatar Nov 19 '20 10:11 TrySound

Hmm, since there is no need for this package in Node.js (right?), couldn't we just switch to just doing ES modules and skip the build step altogether? and release that as a new major version...

It seems that that is much easier for us to maintain, and would still be supported by all bundlers, and modern web browsers without a bundler.

e.g. after this change, when working on the package, I know have to remember to run npm run watch or otherwise I will test old code. And I also run the chance of reloading to quickly in the browser after making a change, and then testing the previous version without my latest changes...

LinusU avatar Nov 19 '20 11:11 LinusU

and would still be supported by all bundlers

Are ESM-only packages supported by browserify?

feross avatar Nov 23 '20 00:11 feross

Are ESM-only packages supported by browserify?

Would you even need to use Browserify at that point? ESM packages are natively supported in the browser. I have seen some posts suggesting you might be able to Babelify with Browserify to use ESM-only packages, but have not tried it.

benmccann avatar Apr 20 '21 16:04 benmccann

You could also support ESM by adding a wrapper which wouldn't require using Rollup or Microbundle: https://nodejs.org/api/packages.html#packages_approach_1_use_an_es_module_wrapper

If we can go ESM-only that'd still be preferable for reduced complexity, but if we need to support both it wouldn't be too hard. Let me know what path forward you'd prefer and I can help

benmccann avatar Apr 20 '21 16:04 benmccann