Declare package type "module"

[ValentinWalter]

Instead of relying on mixed .mjs and .js file extensions (which may confuse some bundlers), declare this package as type "module" and use consistent file extensions.

[gordonnl]

I would need to test all of the different bundlers again, as the reason for .mjs was that a few were ignoring the type module declaration.

[CodyJasonBennett]

This is still no good, main implies that the target is CJS, whereas module is ESM. See #143, no further changes are required. For proper Node resolution as an ESM-only package, you need either type="module" or use the .mjs file extension.

{
  "type": "module",
  "module": "src/index.js"
}
// or,
{
  "module": "src/index.mjs"
}

The first is much more lenient on older build tools and CDNs.

[pschroen]

@CodyJasonBennett, I understand the background on why, or even for backwards compatibility with older versions of Node.js, though this is an ES module library, not a CommonJS library, it's normal to use the main entry point and the recommended approach for ES module packages. From the Node.js docs:

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.

For reference: https://nodejs.org/api/packages.html#dual-commonjses-module-packages

[CodyJasonBennett]

@pschroen, the focus of this issue is enabling Node resolution for compat with bundlers and SSR frameworks. As they stand, main and module are firmly for CJS and ESM targets, respectively. Regarding your comment, is there any case where this is insufficient, depending on the Node version/runtime? I am not sure what the implications of this change are.

[pschroen]

This change will make no difference with how modern versions of Node.js or bundlers work, it's more about having consistent file extensions, and in my experience this no-longer confuses bundlers. For Node.js the behaviour is the same whether you use .mjs or "type": "module". Regarding "main", from the same docs on dual packages:

Node.js can now run ES module entry points

Another way of looking at it is the "module" field is there for backwards compatibility, or dual packages. "main" works fine for ES module only packages, and works both from Node.js directly as the main entry point, and in the latest versions of bundlers (I've tested Node.js, Rollup and webpack). @gordonnl will need to confirm as he mentioned.

I guess the point I'm making here is instead of adding backwards compatibility to this library, I think it would be better to upgrade the older systems. Node.js at-least has supported ES modules natively this way (without an experimental flag) since 2020.

For reference: https://nodejs.org/en/blog/release/v12.17.0/

[CodyJasonBennett]

Awesome, thanks for the info! I'll have to try with esbuild and more obscure tools like metro, but I'd default to preferring compatibility if it's inconsequential with modern versions. I worry of this breaking compat with various transformers and tools that evaluate the main and module field as an indicator for which loader to use (e.g. a testing CLI like Jest or Vitest). This is evidently not a future-proof practice, and something I'll have to forward there.

[pschroen]

Yup, understood. There are a lot of tools to test, I'll leave it with @gordonnl to make a call on compatibility. 😅

And please do share your findings, would like to know how "type": "module" and "main" effects other tools as well. 👍

[gordonnl]

Have updated the package json in 0.0.96 to

"exports": "./src/index.mjs",
"type": "module",

as per instruction in https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c Let me know if it doesn't work in your specific pipelines.

[pschroen]

Hrmm, I don't know about this, it seems like a step backwards to support older systems imho.

Tested this just now with the latest version of Node.js and it's working with the "type": "module" field and ogl@0.0.96, but tree-shaking is broken now in bundlers because "sideEffects": false was removed. 🫠

[gordonnl]

Thanks for testing. Added sideEffects back in 0.0.97. I guess we need someone to re-test in a pipeline that was previously broken to see if any improvements have been made.

[pschroen]

@CodyJasonBennett ☝️ 😉

[CodyJasonBennett]

Hrmm, I don't know about this, it seems like a step backwards to support older systems IMHO.

Tested this just now with the latest version of Node.js and it's working with the "type": "module" field and ogl@0.0.96, but tree-shaking is broken now in bundlers because "sideEffects": false was removed. 🫠

exports was added in Node 12+ and will take precedence over main and module, but it restricts you to entry points defined there. This isn't problematic in OGL's case, but would be a non-starter for something like three. It should be considered a bug in any respective older system that doesn't respect this field (e.g. Metro see https://github.com/facebook/metro/issues/670).

Thanks for testing. Added sideEffects back in 0.0.97. I guess we need someone to re-test in a pipeline that was previously broken to see if any improvements have been made.

I can confirm that tree-shaking works in Webpack again since adding sideEffects. I've also tested between a bunch of tools with different resolution strategies (Webpack 5, Rollup, ESBuild, Vite, Jest 28/Vitest, TSC with moduleResolution=node) and can confirm OGL now works between them and at run-time in Node/SSR.

Notably, Webpack 4 will throw since the usage of nullish coalescing (??) in #148, but this should only affect neglected tools like Bundlephobia (related: https://github.com/pastelsky/bundlephobia/issues/559#issuecomment-907151115). You could also use the .js file extension instead of .mjs since adding type=module, but things should be resolved as-is.

[gordonnl]

Thanks a lot for your diligence @CodyJasonBennett. I'm more than happy to update this if any bundlers are broken, but it seems like they're mostly(all?) happy for the moment with the current setup. Closing for now.