Support both ESM and CJS modules

[AriaMinaei]

This may be trickier than it looks. My first attempt at shipping .mjs and .cjs builds failed as it broke some build systems. For example, shipping an .mjs build with module mapping broke CRA, but not parcel.

Others seem to have similar issues as well:
https://twitter.com/sebmarkbage/status/1498850329156329472
https://twitter.com/_rschristian/status/1498859530444320772
preactjs/preact#3220 (comment)

[AriaMinaei]

Related: #59 and #99

[AriaMinaei]

So, I just took a look at the build pipeline, and I think it has enough documentation to explain how it works. I suggest starting with the yarn build script to see how each package's build script works.

[donmccurdy]

For what it's worth, I've found this to be enough of a pain that I'm trying to only ship ESM in new libraries, particularly libraries meant for web. I think three.js will eventually drop its pseudo-CJS builds as well. Unless I need separate web / node.js builds, then I might do the node.js version as CJS.

If you do go forward though, I've found microbundle's defaults to be quite helpful.

[rschristian]

Came across this by chance (always funny seeing myself quoted in the wild), but certainly feel free to ping if you ever need help pushing it forward.

Just some forewarning, NextJS in particular will always be difficult as it frequently invokes the dual package hazard. This is something that routinely breaks every few months so shipping dual module output does often require some push back on users and telling them to go file upstream unfortunately.

Microbundle's recommendation works with every modern build tool, but it would probably be better to keep package type as CJS and instead ship .mjs (if using "exports", shipping .mjs for "module" risks breakages for no real gain as that field does not need to be bound by Node semantics) if you'd like to support older build tools. .mjs was picked up and popularized before .cjs, so there are some tools that support only the former.

[vezwork]

For 0.5 we are going to support CJS for Core and Studio; and both CJS and ESM for the r3f extension. I tested this configuration of packages with NextJS (it successfully avoid the dual package hazard that @rschristian mentioned), Parcel, Vite, and Create React App.

---

I attempted, but failed to add ESM support to Core and Studio using ESM wrappers for CJS. I failed to fix the dual package issue in NextJS when both ESM and CJS exports are available.

[donmccurdy]

Is Next.js still blocking ESM support? If it's mainly Next.js holding things back, and compiling CJS/ESM builds isn't a problem otherwise, perhaps it would be possible to create a second entrypoint:

// package.json
"type": "module",
"types": "./dist/index.d.ts",
"main": "./dist/cjs/index.cjs",
"exports": {
  "./cjs": {
    "types": "./dist/index.d.ts",
    "default": "./dist/cjs/index.cjs"
  },
  "types": "./dist/index.d.ts",
  "require": "./dist/cjs/index.cjs",
  "default": "./dist/modern/index.js",
}

Then Next.js users can opt into the CJS entrypoint, and other users can have a more modern installation. Or the reverse of that, with an opt-in ./modern entrypoint. I'm having some trouble getting @theatre/core to interact well with some ESM environments, particularly where @theatre/core is a dependency of a dependency.

[AriaMinaei]

Hey @donmccurdy, is it possible to share a repro of this? We'd then put it in /compat-tests and fix it.

[donmccurdy]

Thanks @AriaMinaei! See #453.

[donmccurdy]

In addition to the test failure in #453, I'm also seeing builds where:

// ✅ ok
import * as theatre from '@theatre/core';
const { getProject, onChange } = theatre;

// ❌ fails
import { getProject, onChange } from "@theatre/core";

Which you'd sure think would be equivalent, but the second line prints this error:

import { getProject, onChange } from "@theatre/core";
         ^^^^^^^^^^
SyntaxError: Named export 'getProject' not found. The requested module '@theatre/core'
is a CommonJS module, which may not support all module.exports as named exports. CommonJS
modules can always be imported via the default export, for example using:

import pkg from '@theatre/core';
const { getProject, onChange } = pkg;

I'm not sure yet how to reduce that to a simple test, though. I am using Vite and SvelteKit SSR. Perhaps that part is expected, while the builds are CJS.

[AriaMinaei]

Thanks for the PR.

And yeah, this is interesting. import {getProject} from "@theatre/core" seems to work with vite2/cra/parcel/next, but doesn't with vite4. I don't know if this is a particular version of vite4 that's failing. We do have a vite4 test that worked as of 0.7. I'll run this test on the 0.7 commit and see if it still works.

[grischaerbe]

Any news on that @AriaMinaei? We're unfortunately still facing this issue with @threlte/theatre and vite 5. There seems to be no configuration that works with vite 5 and these combinations. If something works in CSR, it doesn't work in SSR and vice-versa.

import * as pkg from '@theatre/core'
const { getProject } = pkg.default
const { getProject } = pkg
const getProject = typeof window !== 'undefined' ? pkg.getProject : pkg.default.getProject // worked in vite 4

import pkg from '@theatre/core'
const { getProject } = pkg // suggested by vite error

import { getProject } from '@theatre/core'

All tested with and without externalizing @theatre/core in the vite config.

Edit: Solved as seen in this comment.

[AriaMinaei]

@grischaerbe 0.8 will fix this. But from what I remember from our chat, you mentioned that this particular problem was a bundler misconfiguration issue, right?

Either way, the fix is merged in 0.8 so it'll be in the next release.

[grischaerbe]

@grischaerbe 0.8 will fix this. But from what I remember from our chat, you mentioned that this particular problem was a bundler misconfiguration issue, right?

Either way, the fix is merged in 0.8 so it'll be in the next release.

This changed again with vite 5 which is the default for new SvelteKit projects. I can't seem to find a configuration that works at all unfortunately, usual CJS/ESM can of worms 🫠

Great to hear that this is fixed in 0.8, is there a timeline or even a canary release for that?

[grischaerbe]

@AriaMinaei I managed to get it working with this intermediate theatre export / barrel file. It's cumbersome, but solves the immediate issue. Thanks for the heads up and sorry for the confusion 👍

[jcheese1]

@AriaMinaei any news on this? struggling as well.