From d1e6d7dc03644bf99c2882622675413221c6f035 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Mon, 31 Aug 2026 11:22:44 -0600 Subject: [PATCH 1/9] fix: Compile TS to JS during packaging --- .npmignore | 5 ++ package.json | 5 +- tasks/test_node_resolve.mjs | 78 ++++++++++++++++++++++++++++++++ tasks/util/node_resolve_probe.js | 26 +++++++++++ tsconfig.build.json | 32 +++++++++++++ 5 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 tasks/test_node_resolve.mjs create mode 100644 tasks/util/node_resolve_probe.js create mode 100644 tsconfig.build.json diff --git a/.npmignore b/.npmignore index b340f1e4467..9b2732ef05d 100644 --- a/.npmignore +++ b/.npmignore @@ -14,3 +14,8 @@ stackgl_modules/node_modules tasks test topojson + +# Exclude the TypeScript files (but not declarations) because Node doesn't +# parse TS when installed in node_modules. +src/**/*.ts +!src/**/*.d.ts diff --git a/package.json b/package.json index b2895d91a04..57484224994 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "test-syntax": "tsx tasks/test_syntax.js && npm run find-strings -- --no-output", "test-bundle": "node tasks/test_bundle.js", "test-plain-obj": "node tasks/test_plain_obj.mjs", + "test-node-resolve": "node tasks/test_node_resolve.mjs", "test": "npm run test-jasmine -- --nowatch && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint", "b64": "python3 test/image/generate_b64_mocks.py && node devtools/test_dashboard/server.mjs", "mathjax3": "node devtools/test_dashboard/server.mjs --mathjax3", @@ -63,7 +64,9 @@ "preversion": "check-node-version --node 22 --npm 10 && npm-link-check && npm ls --prod --all", "version": "npm run build && git add -A lib dist build src/version.js", "postversion": "node -e \"console.log('Version bumped and committed. If ok, run: git push && git push --tags')\"", - "postpublish": "node tasks/sync_packages.js" + "postpublish": "node tasks/sync_packages.js", + "prepack": "tsc -b tsconfig.build.json --force", + "postpack": "tsc -b tsconfig.build.json --clean" }, "dependencies": { "@plotly/d3": "3.8.2", diff --git a/tasks/test_node_resolve.mjs b/tasks/test_node_resolve.mjs new file mode 100644 index 00000000000..087cd0129f8 --- /dev/null +++ b/tasks/test_node_resolve.mjs @@ -0,0 +1,78 @@ +import { execFileSync } from 'node:child_process'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +import { pathToRoot } from './util/constants.js'; + +// Bundlers resolve a `.ts` extension, so `npm run build` hides a package that +// Node alone cannot load. This test packs the real tarball and loads it the way +// a Node consumer does: `require('plotly.js')` under the CommonJS resolver. +// See https://github.com/plotly/plotly.js/issues/7995. +// +// The package needs a browser, so the load always ends in a DOM error. That is +// the pass condition. Any resolution error is the regression. + +// tsc overwrites a hand-written `foo.js` when a `foo.ts` sits beside it, and it +// reports no error. Such a pair is already ambiguous, because esbuild picks the +// `.ts` and the local build silently ignores the `.js`. Fail here instead. +const collisions = fs + .globSync('src/**/*.ts', { cwd: pathToRoot }) + .filter((file) => !file.endsWith('.d.ts')) + .filter((file) => fs.existsSync(path.join(pathToRoot, file.replace(/\.ts$/, '.js')))); + +if (collisions.length) { + throw new Error( + [ + 'A TypeScript source shares a basename with a JavaScript file:', + ...collisions.map((file) => ' ' + file), + 'The pack step would overwrite the JavaScript file. Rename one of the two.' + ].join('\n') + ); +} + +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'plotly-node-resolve-')); + +try { + console.log('Packing the tarball'); + const packed = execFileSync('npm', ['pack', '--pack-destination', tmp, '--silent'], { + cwd: pathToRoot, + encoding: 'utf8' + }) + .trim() + .split('\n') + .pop(); + + // Install the tarball the way npm would, so that `require('plotly.js')` + // goes through the package name, the `main` field, and the published file + // layout. + const pkg = path.join(tmp, 'node_modules', 'plotly.js'); + + fs.mkdirSync(pkg, { recursive: true }); + execFileSync('tar', ['-xzf', path.join(tmp, packed), '-C', pkg, '--strip-components=1']); + + // The tarball carries no dependencies. Borrow the ones already installed. + fs.symlinkSync(path.join(pathToRoot, 'node_modules'), path.join(pkg, 'node_modules'), 'dir'); + + // The probe resolves from `tmp`, which is where the tarball is installed. + // It runs in its own process so that it starts with a clean module registry + // and its own globals. + const probe = path.join(pathToRoot, 'tasks', 'util', 'node_resolve_probe.js'); + const result = execFileSync(process.execPath, [probe, tmp], { encoding: 'utf8' }).trim(); + + // A ReferenceError means every `require` in the graph resolved, and the + // package only then reached for a browser API. + if (result === 'LOADED' || result === 'RUNTIME:ReferenceError') { + console.log('OK: the published package resolves under Node (' + result + ')'); + } else { + throw new Error( + [ + 'The published package does not resolve under Node: ' + result, + 'Every src/**/*.ts needs a generated .js sibling in the tarball.', + 'See tsconfig.build.json and the prepack script in package.json.' + ].join('\n') + ); + } +} finally { + fs.rmSync(tmp, { recursive: true, force: true }); +} diff --git a/tasks/util/node_resolve_probe.js b/tasks/util/node_resolve_probe.js new file mode 100644 index 00000000000..934d0783c8b --- /dev/null +++ b/tasks/util/node_resolve_probe.js @@ -0,0 +1,26 @@ +// Loads plotly.js the way a Node consumer does. +// +// Takes the directory holding a `node_modules` with the packed tarball in it. +// `createRequire` bases resolution there, so the require below behaves as if +// this file sat in that directory: it goes through the package name, the `main` +// field, and the published file layout. +// +// plotly.js needs a browser, so even a complete load ends in a DOM error. The +// caller reads the single line this prints on stdout. + +const { createRequire } = require('node:module'); +const path = require('node:path'); + +const consumerDir = process.argv[2]; +const consumerRequire = createRequire(path.join(consumerDir, 'index.js')); + +globalThis.self = globalThis; +globalThis.window = globalThis; + +try { + consumerRequire('plotly.js'); + console.log('LOADED'); +} catch (err) { + console.log(err.code === undefined ? 'RUNTIME:' + err.name : 'CODE:' + err.code); + console.error(err.message.split('\n')[0]); +} diff --git a/tsconfig.build.json b/tsconfig.build.json new file mode 100644 index 00000000000..0d660e53b3f --- /dev/null +++ b/tsconfig.build.json @@ -0,0 +1,32 @@ +{ + // Emit configuration for the published package. + // + // The repository authors a growing share of `src/` in TypeScript, but the + // published package must contain only JavaScript. Node's CommonJS resolver + // never tries a `.ts` extension, and Node refuses to strip types from any + // file below `node_modules`. So the `prepack` script writes a `.js` sibling + // for each `.ts` source, and `postpack` deletes it again. + // + // No `outDir` is set, so each `.js` lands next to its `.ts`. That is what + // makes `require('./mod')` resolve in the tarball. + // + // Build mode drives both scripts. `tsc -b` emits, and `tsc -b --clean` + // removes every generated file. Build mode also writes a state file, which + // `tsBuildInfoFile` parks below `build/`, because `build/` is already + // ignored by both git and npm. + // + // Type errors are not reported here. `npm run typecheck` owns that job and + // reads the whole program, including the JavaScript files. + "extends": "./tsconfig.json", + "compilerOptions": { + "noEmit": false, + "noCheck": true, + "allowJs": false, + "module": "commonjs", + "declaration": false, + "isolatedModules": false, + "tsBuildInfoFile": "build/ts-build-info.json" + }, + "include": ["src/**/*.ts"], + "exclude": ["src/types/**"] +} From fa80f38898c13b3d5c0228578a1113091249aa89 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Mon, 31 Aug 2026 11:23:43 -0600 Subject: [PATCH 2/9] Add CI step to test Node resolution --- .github/workflows/ci.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 918b9122cc5..712579f42dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -596,6 +596,15 @@ jobs: - name: Verify generated types are in sync with schema run: npm run schema-typegen-diff-check + package-resolution: + needs: install-and-cibuild + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 + - uses: ./.github/actions/setup-workspace + - name: Verify the published package loads under Node + run: npm run test-node-resolve + # ============================================================ # Standalone jobs (no dependencies on install-and-cibuild) # ============================================================ From f1951790436dccd5c458ea9ad03e65f3f544bfe0 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Mon, 31 Aug 2026 11:24:45 -0600 Subject: [PATCH 3/9] fix: Reference correct Data type --- lib/index.d.ts | 6 ------ src/types/core/data.internal.d.ts | 2 +- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/index.d.ts b/lib/index.d.ts index ceaae5d7930..e17f8128ec8 100644 --- a/lib/index.d.ts +++ b/lib/index.d.ts @@ -61,12 +61,6 @@ export type { YAxisName } from '../src/types/core/layout'; -// --------------------------------------------------------------------------- -// Trace data -// --------------------------------------------------------------------------- - -export type { Data } from '../src/types/core/data'; - // --------------------------------------------------------------------------- // Configuration // --------------------------------------------------------------------------- diff --git a/src/types/core/data.internal.d.ts b/src/types/core/data.internal.d.ts index 10e9036c72f..9531ae04fae 100644 --- a/src/types/core/data.internal.d.ts +++ b/src/types/core/data.internal.d.ts @@ -5,8 +5,8 @@ * properties. For public trace types, see data.d.ts. */ +import type { Data } from '../generated/schema'; import type { Datum } from '../lib/common'; -import type { Data } from './data'; /** * Calculated trace data (internal). From 66ad2624bad217746b9abe40e0fb4dc66688b88b Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Mon, 31 Aug 2026 11:37:17 -0600 Subject: [PATCH 4/9] fix: Update d3 types and move them to dependency --- package-lock.json | 9 ++++----- package.json | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/package-lock.json b/package-lock.json index d54fc2b80a3..6ccab7df294 100644 --- a/package-lock.json +++ b/package-lock.json @@ -16,6 +16,7 @@ "@turf/area": "^7.3.5", "@turf/centroid": "^7.3.5", "@turf/meta": "^7.3.5", + "@types/d3": "^3.5.53", "base64-arraybuffer": "^1.0.2", "country-iso-search": "^0.1.2", "culori": "^4.0.2", @@ -54,7 +55,6 @@ "@biomejs/biome": "^2.5.5", "@plotly/mathjax-v3": "npm:mathjax@^3.2.2", "@plotly/mathjax-v4": "npm:mathjax@^4.1.3", - "@types/d3": "3.5.34", "@types/node": "^26.1.1", "assert": "^2.1.0", "buffer": "^6.0.3", @@ -1540,10 +1540,9 @@ } }, "node_modules/@types/d3": { - "version": "3.5.34", - "resolved": "https://registry.npmjs.org/@types/d3/-/d3-3.5.34.tgz", - "integrity": "sha512-2Ub7NdmaSLviC8lwRGt/7use4LBdLQi7iPEkM97yGKrbmCUqepOgOrGJLi1jPdR0/IIwBDpIbtOgdAOJWWXC+Q==", - "dev": true, + "version": "3.5.53", + "resolved": "https://registry.npmjs.org/@types/d3/-/d3-3.5.53.tgz", + "integrity": "sha512-8yKQA9cAS6+wGsJpBysmnhlaaxlN42Qizqkw+h2nILSlS+MAG2z4JdO6p+PJrJ+ACvimkmLJL281h157e52psQ==", "license": "MIT" }, "node_modules/@types/geojson": { diff --git a/package.json b/package.json index 57484224994..d9704e581a3 100644 --- a/package.json +++ b/package.json @@ -76,6 +76,7 @@ "@turf/area": "^7.3.5", "@turf/centroid": "^7.3.5", "@turf/meta": "^7.3.5", + "@types/d3": "^3.5.53", "base64-arraybuffer": "^1.0.2", "country-iso-search": "^0.1.2", "culori": "^4.0.2", @@ -114,7 +115,6 @@ "@biomejs/biome": "^2.5.5", "@plotly/mathjax-v3": "npm:mathjax@^3.2.2", "@plotly/mathjax-v4": "npm:mathjax@^4.1.3", - "@types/d3": "3.5.34", "@types/node": "^26.1.1", "assert": "^2.1.0", "buffer": "^6.0.3", From e1089f84540a1a81817c7fee7473d6a1ba8b18cb Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Mon, 31 Aug 2026 12:55:17 -0600 Subject: [PATCH 5/9] Remove unneeded tsconfig options --- tsconfig.build.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tsconfig.build.json b/tsconfig.build.json index 0d660e53b3f..ee48c7dcc71 100644 --- a/tsconfig.build.json +++ b/tsconfig.build.json @@ -23,9 +23,7 @@ "noCheck": true, "allowJs": false, "module": "commonjs", - "declaration": false, - "isolatedModules": false, - "tsBuildInfoFile": "build/ts-build-info.json" + "tsBuildInfoFile": "build/tsconfig.build.tsbuildinfo" }, "include": ["src/**/*.ts"], "exclude": ["src/types/**"] From f945b67e8643ed9d4f6e114ef5407a534cdaff5b Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Mon, 31 Aug 2026 12:59:44 -0600 Subject: [PATCH 6/9] Add draftlog --- draftlogs/8000_fix.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 draftlogs/8000_fix.md diff --git a/draftlogs/8000_fix.md b/draftlogs/8000_fix.md new file mode 100644 index 00000000000..2e3df165607 --- /dev/null +++ b/draftlogs/8000_fix.md @@ -0,0 +1 @@ +- Compile TypeScript files under `src/` to JavaScript during packaging to fix Node resolution [[#8000](https://github.com/plotly/plotly.js/pull/8000)] From 8035cf98e0d9ef426cea7b8f7f166d9424b97bb6 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 1 Sep 2026 07:10:26 -0600 Subject: [PATCH 7/9] Update docstring --- src/types/core/data.internal.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/core/data.internal.d.ts b/src/types/core/data.internal.d.ts index 9531ae04fae..e1483f60fcd 100644 --- a/src/types/core/data.internal.d.ts +++ b/src/types/core/data.internal.d.ts @@ -2,7 +2,7 @@ * Internal data/trace types (not in public API) * * These are runtime-resolved versions of trace data with internal state - * properties. For public trace types, see data.d.ts. + * properties. For public trace types, see generated/schema.d.ts. */ import type { Data } from '../generated/schema'; From c1d910fd35aa9a05126b373ad54dff1ba6196658 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 1 Sep 2026 14:09:03 -0600 Subject: [PATCH 8/9] Remove the local resolve test --- package.json | 1 - tasks/test_node_resolve.mjs | 78 -------------------------------- tasks/util/node_resolve_probe.js | 26 ----------- 3 files changed, 105 deletions(-) delete mode 100644 tasks/test_node_resolve.mjs delete mode 100644 tasks/util/node_resolve_probe.js diff --git a/package.json b/package.json index d9704e581a3..40ffca7239a 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,6 @@ "test-syntax": "tsx tasks/test_syntax.js && npm run find-strings -- --no-output", "test-bundle": "node tasks/test_bundle.js", "test-plain-obj": "node tasks/test_plain_obj.mjs", - "test-node-resolve": "node tasks/test_node_resolve.mjs", "test": "npm run test-jasmine -- --nowatch && npm run test-bundle && npm run test-image && npm run test-export && npm run test-syntax && npm run lint", "b64": "python3 test/image/generate_b64_mocks.py && node devtools/test_dashboard/server.mjs", "mathjax3": "node devtools/test_dashboard/server.mjs --mathjax3", diff --git a/tasks/test_node_resolve.mjs b/tasks/test_node_resolve.mjs deleted file mode 100644 index 087cd0129f8..00000000000 --- a/tasks/test_node_resolve.mjs +++ /dev/null @@ -1,78 +0,0 @@ -import { execFileSync } from 'node:child_process'; -import fs from 'node:fs'; -import os from 'node:os'; -import path from 'node:path'; - -import { pathToRoot } from './util/constants.js'; - -// Bundlers resolve a `.ts` extension, so `npm run build` hides a package that -// Node alone cannot load. This test packs the real tarball and loads it the way -// a Node consumer does: `require('plotly.js')` under the CommonJS resolver. -// See https://github.com/plotly/plotly.js/issues/7995. -// -// The package needs a browser, so the load always ends in a DOM error. That is -// the pass condition. Any resolution error is the regression. - -// tsc overwrites a hand-written `foo.js` when a `foo.ts` sits beside it, and it -// reports no error. Such a pair is already ambiguous, because esbuild picks the -// `.ts` and the local build silently ignores the `.js`. Fail here instead. -const collisions = fs - .globSync('src/**/*.ts', { cwd: pathToRoot }) - .filter((file) => !file.endsWith('.d.ts')) - .filter((file) => fs.existsSync(path.join(pathToRoot, file.replace(/\.ts$/, '.js')))); - -if (collisions.length) { - throw new Error( - [ - 'A TypeScript source shares a basename with a JavaScript file:', - ...collisions.map((file) => ' ' + file), - 'The pack step would overwrite the JavaScript file. Rename one of the two.' - ].join('\n') - ); -} - -const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'plotly-node-resolve-')); - -try { - console.log('Packing the tarball'); - const packed = execFileSync('npm', ['pack', '--pack-destination', tmp, '--silent'], { - cwd: pathToRoot, - encoding: 'utf8' - }) - .trim() - .split('\n') - .pop(); - - // Install the tarball the way npm would, so that `require('plotly.js')` - // goes through the package name, the `main` field, and the published file - // layout. - const pkg = path.join(tmp, 'node_modules', 'plotly.js'); - - fs.mkdirSync(pkg, { recursive: true }); - execFileSync('tar', ['-xzf', path.join(tmp, packed), '-C', pkg, '--strip-components=1']); - - // The tarball carries no dependencies. Borrow the ones already installed. - fs.symlinkSync(path.join(pathToRoot, 'node_modules'), path.join(pkg, 'node_modules'), 'dir'); - - // The probe resolves from `tmp`, which is where the tarball is installed. - // It runs in its own process so that it starts with a clean module registry - // and its own globals. - const probe = path.join(pathToRoot, 'tasks', 'util', 'node_resolve_probe.js'); - const result = execFileSync(process.execPath, [probe, tmp], { encoding: 'utf8' }).trim(); - - // A ReferenceError means every `require` in the graph resolved, and the - // package only then reached for a browser API. - if (result === 'LOADED' || result === 'RUNTIME:ReferenceError') { - console.log('OK: the published package resolves under Node (' + result + ')'); - } else { - throw new Error( - [ - 'The published package does not resolve under Node: ' + result, - 'Every src/**/*.ts needs a generated .js sibling in the tarball.', - 'See tsconfig.build.json and the prepack script in package.json.' - ].join('\n') - ); - } -} finally { - fs.rmSync(tmp, { recursive: true, force: true }); -} diff --git a/tasks/util/node_resolve_probe.js b/tasks/util/node_resolve_probe.js deleted file mode 100644 index 934d0783c8b..00000000000 --- a/tasks/util/node_resolve_probe.js +++ /dev/null @@ -1,26 +0,0 @@ -// Loads plotly.js the way a Node consumer does. -// -// Takes the directory holding a `node_modules` with the packed tarball in it. -// `createRequire` bases resolution there, so the require below behaves as if -// this file sat in that directory: it goes through the package name, the `main` -// field, and the published file layout. -// -// plotly.js needs a browser, so even a complete load ends in a DOM error. The -// caller reads the single line this prints on stdout. - -const { createRequire } = require('node:module'); -const path = require('node:path'); - -const consumerDir = process.argv[2]; -const consumerRequire = createRequire(path.join(consumerDir, 'index.js')); - -globalThis.self = globalThis; -globalThis.window = globalThis; - -try { - consumerRequire('plotly.js'); - console.log('LOADED'); -} catch (err) { - console.log(err.code === undefined ? 'RUNTIME:' + err.name : 'CODE:' + err.code); - console.error(err.message.split('\n')[0]); -} From d0454f75b4bb88bdaf87459965a578f9d2dbdc34 Mon Sep 17 00:00:00 2001 From: Cameron DeCoster Date: Tue, 1 Sep 2026 14:28:51 -0600 Subject: [PATCH 9/9] Perform resolve test in CI --- .github/workflows/ci.yml | 45 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 712579f42dc..f36627ac9dd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -602,8 +602,49 @@ jobs: steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6 - uses: ./.github/actions/setup-workspace - - name: Verify the published package loads under Node - run: npm run test-node-resolve + + - name: Pack the package + run: npm pack --pack-destination "$RUNNER_TEMP" + + - name: Install the tarball into a scratch project + working-directory: ${{ runner.temp }} + run: | + mkdir consumer && cd consumer + npm init -y + npm install "$RUNNER_TEMP"/plotly.js-*.tgz + + - name: Load every compiled module under Node + shell: node {0} + working-directory: ${{ runner.temp }}/consumer + run: | + const assert = require('node:assert'); + const fs = require('node:fs'); + + // src/lib/index.js reaches for these before it touches the DOM. + globalThis.self = globalThis; + globalThis.window = globalThis; + + const paths = [process.cwd()]; + const load = (name) => require(require.resolve(name, { paths })); + + const modules = fs + .globSync('src/**/*.ts', { cwd: process.env.GITHUB_WORKSPACE }) + .filter((file) => !file.endsWith('.d.ts')) + .map((file) => 'plotly.js/' + file.replace(/[.]ts$/, '')); + + if (modules.length === 0) throw new Error('Found no TypeScript sources to check'); + + for (const name of modules) load(name); + + const lib = load('plotly.js/src/lib/index'); + + assert.strictEqual(lib.mod(-1, 4), 3); + assert.strictEqual(lib.modHalf(3, 4), -1); + assert.deepStrictEqual(lib.sortObjectKeys({ b: 1, a: 2 }), ['a', 'b']); + assert.strictEqual(lib.cleanNumber(' 12 '), 12); + assert.strictEqual(typeof lib.counterRegex, 'function'); + + console.log('Loaded ' + modules.length + ' compiled modules and src/lib/index.js'); # ============================================================ # Standalone jobs (no dependencies on install-and-cibuild)