Skip to content

fix: resolve #standard-fonts/* to one file under every condition - #1782

Open
mendrinos wants to merge 1 commit into
foliojs:masterfrom
mendrinos:fix/standard-fonts-import-condition
Open

fix: resolve #standard-fonts/* to one file under every condition#1782
mendrinos wants to merge 1 commit into
foliojs:masterfrom
mendrinos:fix/standard-fonts-import-condition

Conversation

@mendrinos

Copy link
Copy Markdown

Problem

js/pdfkit.node.mjs loads the standard font metrics lazily through createRequire:

const require$1 = createRequire(import.meta.url);
const STANDARD_FONTS = {
  Helvetica: () => require$1('#standard-fonts/Helvetica'),
  ...
};

package.json#imports maps that specifier two ways:

"#standard-fonts/*": {
  "require": "./js/standard-fonts/*.cjs",
  "default": "./js/standard-fonts/*.mjs"
}

The call is a require, so at runtime Node and Bun both load
js/standard-fonts/Helvetica.cjstests/package-resolution.mjs already asserts
exactly that. But a bundler or file tracer walks the same file as ESM and resolves
the same specifier under default, landing on the .mjs twin. The two disagree,
and for anything that packages a traced dependency set, the tracer wins.

With @vercel/nft — the tracer behind Vercel,
Nitro/Nuxt and @vercel/ncc — tracing an ESM entry that imports pdfkit collects all
14 js/standard-fonts/*.mjs and none of the .cjs files:

import { nodeFileTrace } from '@vercel/nft'
// entry.mjs: `import PDFDocument from 'pdfkit'; export default PDFDocument`
const { fileList } = await nodeFileTrace(['entry.mjs'])
;[...fileList].filter(f => f.includes('standard-fonts'))
// → every *.mjs, no *.cjs

The build succeeds and the package looks complete in the output. Then the first
document throws:

Cannot find module '#standard-fonts/Helvetica' from '/app/.output/server/node_modules/pdfkit/js/pdfkit.node.mjs'

Reproducible by copying that traced file list into a directory with no ancestor
node_modules and constructing a PDFDocument — fails under both Node 23 and Bun 1.4.

Fix

Nothing reachable at runtime takes the import condition here. The browser builds
register fonts through registerStdFonts and never reference #standard-fonts
(grep -c standard-fonts js/pdfkit.browser.mjs → 0); the only consumer is
lib/document.node.js, reached by require from both node builds. The default
branch is therefore unreachable and serves only to point resolvers at a file pdfkit
never loads.

"#standard-fonts/*": "./js/standard-fonts/*.cjs"

Runtime behaviour is unchanged — same file, still lazy, still one font at a time.
Tracers now resolve what Node resolves, and traced output gets slightly smaller
because the unused ESM twins are no longer dragged along.

The public ./standard-fonts/* export is untouched: consumers who import 'pdfkit/standard-fonts/Helvetica' still get the ESM build, and rollup still emits
both variants.

Alternative

If the conditional mapping should stay, the other way out is to have the build emit
explicit relative specifiers (require('./standard-fonts/Helvetica.cjs')) in the node
ESM bundle — unambiguous to every analyser, no condition resolution involved. Happy
to redo it that way if you prefer; it is a rollup-config change rather than a manifest
one.

Test

tests/package-resolution.mjs gains an assertion that both conditions land on the
same file. It fails on master with the exact shape of the bug:

+ actual   - expected
+ '.../js/standard-fonts/Helvetica.mjs'
- '.../js/standard-fonts/Helvetica.cjs'

yarn test:package, yarn test:unit (432 passing), yarn lint and yarn prettier
are all green with the change.

Context

Hit downstream on a Nuxt/Nitro app: receipt PDFs quietly stopped being attached to
payment emails (the generation error was caught and logged) and every other PDF route
500'd, while CI stayed green — nothing in a normal test run exercises the packaged
output. Same class as #1779, one level further down.

The node ESM build loads the standard font metrics lazily through
`createRequire`, so the require condition is the only one ever taken at
runtime. Bundlers and file tracers walk `js/pdfkit.node.mjs` as ESM and
resolve the same specifier under the import condition, landing on the
`.mjs` twin instead. Anything that packages a traced dependency set ships
the modules pdfkit never loads and omits the ones it does, and the first
`new PDFDocument()` throws `Cannot find module '#standard-fonts/Helvetica'`.

Nothing reachable at runtime uses the import condition here: the browser
builds register their fonts through `registerStdFonts` and never reference
`#standard-fonts`, and both node builds arrive via `require`. Point the
internal mapping at the CommonJS files so every resolver agrees with Node.

The public `./standard-fonts/*` export is untouched, so consumers importing
`pdfkit/standard-fonts/Helvetica` still get the ESM build.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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

Successfully merging this pull request may close these issues.

1 participant