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

Separate/more granular dist targets #951

Closed
ersinakinci opened this issue May 15, 2020 · 12 comments
Closed

Separate/more granular dist targets #951

ersinakinci opened this issue May 15, 2020 · 12 comments

Comments

@ersinakinci
Copy link

It's really convenient to directly import Hyperapp via Unpkg from inside a <script type="module"> tag. At present, however, importing directly from Unpkg pulls in src/index.js, which isn't minified. This behavior is a problem because we don't get the chance to minify using a bundler when importing directly within a script tag.

Furthermore, dist/index.js is currently built using IIFE, whereas UMD would be much more flexible (e.g., if you want to import Hyperapp in a Node environment).

I propose that package.json point to three targets:

  • "main": "dist/index.js": UMD target. This file is currently built using IIFE, whereas switching to UMD will allow the file to be imported as a CommonJS/AMD and falls back to IIFE in a browser environment. UMD may increase the bundle size vs. pure IIFE, however.
  • "module": "src/index.js": ES modules target.
  • "unpkg": "dist/index.es.min.js": Minified ES modules target.
@jorgebucaran
Copy link
Owner

if you want to import Hyperapp in a Node environment

Node now supports ES modules.

@jorgebucaran
Copy link
Owner

What about doing as you say for module and unpkg, and continue with IIFE for "main"?

@ersinakinci
Copy link
Author

Node now supports ES modules.

True, but only as of 14.x, and even then it's considered experimental/unstable (albeit, enabled by default). Lots of folks are still on 12.x, which is the latest LTS release, or even 10.x.

What about doing as you say for module and unpkg, and continue with IIFE for "main"?

Sure, but what's your concern about using UMD? It still falls back to IIFE for the browser and enables older versions of Node to use the library.

I think it's legitimate to say "we just aren't going to support CommonJS." I did some research and it appears that supporting both CommonJS and ES modules can lead to some subtle problems in Node, which can be avoided/mitigated but would require some commitment (see the link).

If we make a policy of not supporting CommonJS, then we should be explicit about it and make the following changes:

  • "main": "src/index.js": ES modules target
  • "module": "src/index.js": ES modules target
  • "unpkg": "dist/index.es.min.js": Minified ES modules target
  • "type": "module": Hint to Node that all JS files should be treated as ES modules
  • Separately, generate dist/hyperapp.js and dist/hyperapp.min.js as IIFE targets. They shouldn't be referred to by package.json because they aren't modular in nature, they're purely for legacy browser import style (i.e, <script src="..." />)
  • Documentation on these differences and the fact that CommonJS is not supported

That said, I do want to emphasize that ES modules support is still experimental in Node 14, even if it's enabled by default. The "proper" way, I think, is to handle both CJS and ESM, but it might not be necessary.

@ersinakinci
Copy link
Author

Assuming that Hyperapp's modules are stateless and will remain stateless, IMHO the most comprehensive configuration would look like:

  • "main": "dist/index.cjs": CommonJS modules target (fallback for Node 10 and below)
  • "module": "src/index.js": ES modules target (preferred over main by Webpack, Rollup, etc., and ignored by Node)
  • "exports": { "import": "src/index.js", "require": "dist/index.cjs" }": Hint to Node 12 and above to use ESM when writing import and CJS when writing require.
  • "unpkg": "dist/index.es.min.js": Minified ES modules target (hint for unpkg, ignored by everything else)
  • "type": "module": Hint to Node 12 and above that all JS files should be treated as ES modules
  • Separately, generate dist/hyperapp.js and dist/hyperapp.min.js as IIFE targets. They shouldn't be referred to by package.json because they aren't modular in nature, they're purely for legacy browser import style (i.e, <script src="..." />)
  • Documentation on these differences and that CommonJS is supported

@jorgebucaran
Copy link
Owner

I agree with all the points you listed in #951 (comment) and that's exactly my plan in time for the official V2 release, you just clarified a bunch of stuff, thanks.

Documentation on these differences and the fact that CommonJS is not supported

Sure thing, we'll do by then.

We just aren't going to support CommonJS.

That's my plan.

@ersinakinci
Copy link
Author

Sounds good :-). I'll close this issue then.

@jorgebucaran
Copy link
Owner

jorgebucaran commented May 16, 2020

@earksiinni Is there any difference between "main" and "module" if both point to an ES module?

I mean this:

"main": "src/index.js"
"unpkg": "dist/index.min.js"
"type": "module"

instead of

"main": "src/index.js"
"module": "src/index.js"
"unpkg": "dist/index.min.js"
"type": "module"

@ersinakinci
Copy link
Author

You're right! To my knowledge, there's no difference if both point to an ES module. Omitting "module" makes more sense.

@jorgebucaran
Copy link
Owner

jorgebucaran commented May 17, 2020

@earksiinni I was looking at unpkg's https://unpkg.com documentation and didn't find a reference to this "unpkg" field in package.json. Is this a thing?

Maybe I found it unpkg/unpkg#93 (comment)

@ersinakinci
Copy link
Author

Ah sorry, I forgot to reply @jorgebucaran. It's specified on unpkg's main page:

If you omit the file path (i.e. use a “bare” URL), unpkg will serve the file specified by the unpkg field in package.json, or fall back to main.

@jaeh
Copy link
Contributor

jaeh commented May 23, 2020

when creating mjs modules please take into account that either "type": "module" has to be set in package.json or the module file has to end with .mjs.

otherwise node will import modules as commonjs files and complain about the import or export statements. this is true even if package.json of the parent project includes "type": "module" and i have not found a workaround that forces node_modules to load as modules by default.

rollup and the other transpilers handle this as one would expect and without errors, node does not.

running into the same issue in the newest version of hyperapp-render: issue

@jorgebucaran
Copy link
Owner

Thanks for all the info @jaeh! 👍

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

No branches or pull requests

3 participants