Build CommonJS module along with ESM and IIFE

[mcrumm]

Brings LiveView JS in line with Phoenix JS in terms of assets exported.
Closes #1654

The only potential concern is that we added main to point to the esm module in #1586 // @felix-starman

[davydog187]

I ran into this issue 10 minutes ago and BOOM PR. Thanks @mcrumm <3

[grossvogel]

This combined with phoenixframework/phoenix#4521 solves all of my build and test issues! Thank you @mcrumm !

[petermueller]

preliminarily it seems to work just fine with our yarn2 based app. I can poke it some more on Monday, but looking at our sourcemap that was generated it grabbed the cjs, and I'm not seeing any esbuild or runtime errors.

[mveytsman]

I think this is right but I wonder if we should be providing a "module" here as well (see https://webpack.js.org/guides/package-exports/#providing-commonjs-and-esm-version-stateless)

[petermueller]

yeah, I was thinking the same thing, although I'm not sure whether the "exports" key has priority over the top-level "module" key or not. I've not seen anything stating which to use, just the "export" is more flexible.

---

TL;DR, it sounds like adding the "exports" top-level along with a "type": "module" is the typical way to support ES modules, and the top-level "module" key I added might not be necessary if we add the "type" key instead.

---

(notes from my reading and digging into the Yarn 2 PnP docs and source)

Okay, if I understand my initial reading on this it sounds like node will treat .cjs and .mjs different when calling import, and .mjs can't be require'd. Setting the extension to .mjs will tell node and modern browsers how to treat it, but then you have to deal with people running into issues where they're doing early development without a bundler and have web server config to only allow js, css, and jpeg, or similar issues that we wouldn't want to force on new folks just trying to get started

"type": "module" sounds like it is the implicit way to do it, and it gives a hint to node/bundlers to treat the import ending in .js as an ES module (i.e. as a .mjs), while still allowing flexibility.

This makes sense to me why we're doing it this way 🤯

This set of articles seems to summarize rather well:
https://dev.to/bnb/conditional-exports-supporting-both-import-and-require-3ihg

[mcrumm]

Okay, so my take-away from all of that is that the configuration in this PR is seemingly correct.

providing a "module" here as well

@mveytsman possibly, but i am hesitant to make further changes :)

[mcrumm]

Btw thanks @felix-starman for the thorough reply!

[mcrumm]

Just to be clear, we are saying that an alternate version of this change (for the relevant) might look like so:

{
  "type": "module",
  "main": "./priv/static/phoenix.cjs.js",
  "unpkg": "./priv/static/phoenix.min.js",
  "jsdelivr": "./priv/static/phoenix.min.js",
  "exports": {
    "import": "./priv/static/phoenix.esm.js",
    "require": "./priv/static/phoenix.cjs.js"
  }
}

is that correct?

[petermueller]

I think so?
I did some testing and at least for my purposes, esbuild likes to default to using the "iife" (immediately invoked function expression) format, and wraps the cjs file so it can do an "import" at runtime. I was only able to force esbuild to use the esm.js by setting it as the "main", which is an odd esbuild thing? (evanw/esbuild#840)

Ultimately though, having the "module" at the top level looks like it is potentially helpful in certain scenarios?
The "main": "...cjs.js" works for Yarn2 + esbuild when following the docs, and assuming there is a way to tell esbuild to prefer the esm.js file over the cjs, it worked just fine then too.

From the general gist, it seems like leaving the "module" and just adding the "type" is just fine too?

{
  "type": "module",
  "module": "./priv/static/phoenix.esm.js",
  "main": "./priv/static/phoenix.cjs.js",
  "unpkg": "./priv/static/phoenix.min.js",
  "jsdelivr": "./priv/static/phoenix.min.js",
  "exports": {
    "import": "./priv/static/phoenix.esm.js",
    "require": "./priv/static/phoenix.cjs.js"
  }
}

I thoroughly dislike JS dependency resolution now 😄

[mcrumm]

Welcome to the party @felix-starman :) and thank you again for the very thorough response!

I've been reading through the links you shared, as well, as the changelog for esbuild, just to get a better sense of things. I think I have that now, so I will share what I have found :)

After digging through the esbuild changelog entries, most notably those in 0.7.0 and 0.10.0, I finally landed on the esbuild docs for main fields which has the most succinct explanation I have yet seen. I will summarize here:

package.json–

• "main" - This is for node, so it's expected to be CommonJS (CJS).
• "module" - This is for ECMAScript modules (ESM), except node uses "type": "module" instead. Also lots of packages incorrectly use module to mean, "code for the browser".
• "browser" - "allows bundlers to replace node-specific files or modules with their browser-friendly versions." I think this has not yet been proposed for Phoenix packages, but perhaps it should be? 🤔

defaults–

"The default main fields depend on the current platform setting and are essentially browser,module,main for the browser and main,module for node. These defaults should be the most widely compatible with the existing package ecosystem. But you can customize them like this if you want to:"

esbuild app.js --bundle --main-fields=module,main

There is so much nuance here. My understanding is that specifying "exports" runs the risk of someone loading both the ESM and CJS bundles into their code, but esbuild does its best to mitigate that? I also get the impression esbuild ignores "exports" entirely, but I am sure other bundlers will rely on it.

My final takeaways:

• The changes in this PR are good to go as-is
• esbuild users can specify the --main-fields flag if they need to change the default
• We should look at replacing "exports" with the top-level "browser" key, but given multiple reports of these settings working for people, I think we save that for another day :)

[mcrumm]

I am leaving this unresolved for posterity :)