feat: add Node ESM build: route the node import condition to a real Node bundle - #1778
Merged
Conversation
diegomura
force-pushed
the
diegomura/helsinki
branch
from
August 23, 2026 14:12
e3fcde6 to
447aeb2
Compare
Collaborator
Author
|
After this I can start using pdfkit in react-pdf 😄 @blikblum I'd appreciate your review and re-publishing 🙏🏻 I think that will be big for this lib |
Member
|
Done. BTW i'm surprised that it will work on ESM in node (due to some dependencies not being ESM friendly) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The root export map only defines
requireunder thenodecondition:When a Node ESM consumer does
import PDFDocument from 'pdfkit', the nestednodeobject has no matching key, so resolution falls through to"default"— and modern Node users silently get the browser build. On Node that means:doc.image('/path.png'),doc.font('/path.ttf'),doc.file('/path')) fail unless the file was pre-registered withregisterFile.zlibSyncinstead of nativezlib.deflateSyncfor every content stream (a measurable perf regression on large documents).Readableshim instead of Node streams.registerStdFonts).Nothing guarded this:
tests/package-resolution.cjsonly exercisesrequire('pdfkit'). (Extra context: react-pdf is converging onto upstream pdfkit and is a pure-ESM consumer, so it currently lands on the browser build in Node.)Change
Add a Node ESM bundle (
js/pdfkit.node.mjs) and wire it into the export map:lib/document.node.jsrollup entry, sharing its plugins, externals and babel config.#fs/#zlib/#streamare inlined via thenodecondition exactly as in the CJS bundle, withfs/zlib/streamexternal.lib/document.node.jsnow gets itsrequirefromcreateRequire(import.meta.url)so the lazy standard-font loaders work in both output formats (rollup shimsimport.meta.urlin the CJS output; the AFM data modules are still only loaded when a standard font is actually used).nodecondition and keep resolving tojs/pdfkit.browser.mjsvia"default".Tests
New
tests/package-resolution.mjs, wired intonpm run test:package:import.meta.resolve('pdfkit')points atjs/pdfkit.node.mjs.tests/package-resolution.cjspreviously usedawait import('pdfkit')to reach the browser bundle (the misresolution this PR fixes); it now asserts the dynamic import yields the Node ESM build and loads the browser ESM bundle directly by path to keep its coverage.npm test(481 tests),npm run test:tools,npm run lintandnpm run prettierall pass.