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

The ES module points to non-transpiled src/core.js file #48

Open
markogresak opened this issue Jul 24, 2020 · 2 comments
Open

The ES module points to non-transpiled src/core.js file #48

markogresak opened this issue Jul 24, 2020 · 2 comments
Assignees
Labels

Comments

@markogresak
Copy link

This module claims support for various older versions of browsers.

But there is an edge case where this is not true.

When used with webpack or similar tools, it's a common practice to use import. Even the docs suggest using

// ES6
import shareThis from "share-this";

But there's a catch. The module resolver prefers module over main in package.json. This is true only for JS Modules (import syntax).

https://github.com/MaxArt2501/share-this/blob/v1.3.1/package.json#L13-L14

  "main": "dist/share-this.js",
  "module": "src/core.js",

In the case of share-this, it means that the import syntax will take the src/core.js. The usual webpack config will not compile anything from node_modules folder for performance reasons.

So when we put all of this together, the result is the raw src/* code landing in the vendor bundle, with const and everything. Internet Explorer says 👎 😄


There are a few possible solutions:

  1. Ship a transpiled dist/share-this.es.js and change the module to point to it.
  2. Remove the module and leave only the main. Most current build pipelines will work with the old commonjs way. Not sure if this will hold true in the future.
  3. Leave it to the consumers to figure it out, either import share-this/dist/share-this or transpile it on their end. It's like this at the moment, so no work required. It's a solution, but a bad one.
@MaxArt2501
Copy link
Owner

I claim no responsibility for what webpack does :| But you can configure it to compile certain modules in node_modules.
Granted that last time I wrote that part in the package.json for this library it wasn't clear at all what the "module" property would have implied, as it is you can do this:

import shareThis from 'https://cdn.jsdelivr.net/npm/share-this/src/core.js';

and that would work perfectly fine.

But I guess I'll look into what's rolling now for those package.json properties, and serve a single file compiled for ES6 in "module"...

@MaxArt2501 MaxArt2501 self-assigned this Jul 25, 2020
@MaxArt2501 MaxArt2501 added the bug label Jul 25, 2020
@markogresak
Copy link
Author

I claim no responsibility for what webpack does

To clarify, it's not just webpack, any "standard" bundler implementation where we use import ... from ... will behave this way. It's even mentioned in the Node.js docs https://nodejs.org/api/esm.html#esm_dual_commonjs_es_module_packages

Prior to the introduction of support for ES modules in Node.js, it was a common pattern for package authors to include both CommonJS and ES module JavaScript sources in their package, with package.json "main" specifying the CommonJS entry point and package.json "module" specifying the ES module entry point. This enabled Node.js to run the CommonJS entry point while build tools such as bundlers used the ES module entry point, since Node.js ignored (and still ignores) the top-level "module" field.

So from this point of view, the use of "module" is not correct, it should be "exports". But it's the de-facto way with bundlers.

Also, the bug I have reported here is the problem node.js docs call "dual package hazard": https://nodejs.org/api/esm.html#esm_dual_package_hazard.

So what I've suggested with option 2), removing the "module" bit sounds like the best option. Or it could be changed to "exports".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants