Skip to content

Package is not "true" ESM + browser ≠ Node.js #3835

Open
@kytta

Description

@kytta

Problem

In its current form, jsPDF suffers from "faux ESM". The package does not use exports field, but opts for the module key in the package.json. This is non-standard; while it is supported by most bundlers, Node.js does not evaluate it and as such will resolve to CommonJS imports instead of ESM:

import { jsPDF } from 'jspdf'; // oops => this imported from jspdf.node.min.js!

This is the behaviour we want, but it's not stable, as you'll see. Either way, it would be cool to also see an ESM build for Node.js, too.

Another issue is that all exported files have the same .js extension. Nowadays, it is not considered valid for dual-style packages, as Node will implicitly evaluate them as whatever type it is that package.json#type declares (or commonjs by default). That means that, even if we declare jspdf.es.min.js as package.json#exports.import, it could still be evaluated as CommonJS and break.

A bigger problem, though, is the fact that the existing ESM bundle is not targeted at Node.js, but at browsers. This introduces a possible peculiar problem for downstream users where, depending on a bundler they use, their final code snippet may not be Node.js-friendly.

Reproduction

Imagine I want to make a Node CLI that uses jsPDF to generate PDF files. For the sake of simplicity, let it generate an empty document. I want to write my code in ESM (because I like it) and bundle it to CommonJS (for better compatibility and to be able to distribute it as a single-file executable). My code is very simple:

import { jsPDF } from "jspdf";

const doc = new jsPDF();
doc.save("empty.pdf");

Let's try bundling it with esbuild:

esbuild --bundle --platform=node --format=cjs index.js --outfile=bin/index.cjs

When using --platform=node, esbuild will import from either main or module. It prefers main, as it expects CommonJS to be there. The final file is Node.js-centric and can save files:

grep 'writeFileSync' bin/index.cjs  # finds usage
node bin/index.cjs                  # exports empty.pdf

Okay, let's try Rollup instead. As per common configuration, we'll use the plugins node-resolve and commonjs:

rollup --format=cjs --file=bin/index.cjs -p node-resolve -p commonjs --inlineDynamicImports -- index.js

Rollup, when seeing ESM imports and when using node-resolve, will import from module. It expects Node-centric ESM to be there, but there is a browser build! The final file is browser-centric and does not work:

grep 'writeFileSync' bin/index.cjs  # ERROR! No such method found
node bin/index.cjs                  # SILENT ERROR! No file is saved

Proposed solution

It's getting more popular to go bundle-less, and it's getting more popular to follow standards, so I think there should be changes to the current package metadata. There are actually two things to do.

Export Node-centric ESM and CJS, and Browser-centric ESM and UMD/IIFE

I propose that, instead of the files we export now, the following is generated in our dist folder
(minified and maps omitted for brevity):

dist
├── jspdf.mjs         => for browsers; use with <script type="module">
├── jspdf.umd.js      => for browsers; use with <script>
├── jspdf.node.cjs    => for Node, includes 'fs'; use with `require()`
├── jspdf.node.mjs    => for Node, includes 'fs'; use with `import`
├── polyfills.mjs     => polyfills; use with <script type="module">
└── polyfills.umd.js  => polyfills; use with <script>

In my opinion, UMD can and should be replaced with IIFE. UMDs are useful for code used by both browsers and Node, but since we're shipping Node-specific code anyway, we could reduce the complexity of the files and export them just as IIFE for the use with <script>. But that's my opinion ;)

This would require us to update the package.json as follows:

{
  "name": "jspdf",
  "main": "./dist/jspdf.node.cjs",
  "module": "./dist/jspdf.node.mjs",
  "browser": "./jspdf.umd.js",
  "exports": {
    "node": {
      "require": "./dist/jspdf.node.cjs",
      "import": "./dist/jspdf.node.mjs"
    },
    "default": "./dist/jspdf.mjs"
  }
}

Please note that this is obviously a breaking change.

Export types for both CJS and ESM

What? Yes! Despite virtually nobody using TypeScript with require(), it's possible, and should be accounted for when providing type definitions. The current types have the .d.ts extension and are technically wrong — .ts is treated as CommonJS (because the default package type is that), but contains ESM (export ...).

To solve this, you'll need to have two different declaration files: One with .d.cts, representing the .cjs, and one with .d.mts, representing the .mjs. Note that this does not impact the behaviour of the package in any way, but it provides a higher value to the developer using it.

Validate

To validate this, I recommend using the arethetypeswrong CLI as well as publint:

npm install
npm run build
npx @arethetypeswrong/cli -P .
npx publint run .

Additional reading

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions