Skip to content

fix: restore the types conditions in the published exports map - #2687

Open
rajwilkhu wants to merge 1 commit into
graphql-hive:masterfrom
rajwilkhu:fix/exports-types-condition
Open

rajwilkhu wants to merge 1 commit into
graphql-hive:masterfrom
rajwilkhu:fix/exports-types-condition

Conversation

@rajwilkhu

Copy link
Copy Markdown

Fixes the problem reported in #2480 and detailed in #2686.

The defect

packages/graphql-modules/package.json has declared the types conditions for
years:

"exports": {
  ".":   { "require": "./dist/index.js", "import": "./dist/index.mjs", "types": "./index.d.ts" },
  "./*": { "require": "./dist/*.js",     "import": "./dist/*.mjs",     "types": "./*.d.ts"     }
}

They never reach npm. bob build rewrites the exports map for the published
dist/ directory in utils/rewrite-exports.js, and that function rebuilds
every entry keeping only two keys:

newValue = {
  require: newValue.require?.replace(`${distDir}/`, ""),
  import:  newValue.import?.replace(`${distDir}/`, ""),
};

Anything else in the condition object — types included — is dropped. So every
published tarball up to and including 3.1.2 ships:

"exports": {
  ".":   { "require": "./index.js", "import": "./index.mjs" },
  "./*": { "require": "./*.js",     "import": "./*.mjs"     },
  "./package.json": "./package.json"
}

which makes the declarations unreachable under moduleResolution bundler,
node16 and nodenext:

error TS7016: Could not find a declaration file for module 'graphql-modules'.
'.../graphql-modules/index.mjs' implicitly has an 'any' type.
  There are types at '.../graphql-modules/index.d.ts', but this result could
  not be resolved when respecting package.json "exports".

Only the legacy node10 resolver works, because it reads the top-level
typings field and ignores exports entirely. That is why #2480 could be
closed without the problem going away, and why it still reproduces on the
latest release.

The fix

The conditions are reinstated in scripts/fix-async-context.js, which already
post-processes dist/package.json for exactly this class of loss — its existing
comment reads "because bob removes the imports field". Putting it there:

  • needs no bob upgrade, which matters because the v7 bump was tried in
    0cc7337 and reverted in 31be509;
  • is driven from the source package.json, so it is a no-op if types is ever
    dropped there, and it cannot invent a path of its own;
  • leaves types first in the condition object, which is the documented order.

Happy to move it into a dedicated script, or to rename that script now that it
does two things, if you'd prefer.

Verification

Measured end to end, not by inspecting the JSON:

Step Result
yarn build on master, then read dist/package.json no types condition
yarn build with this change { types, require, import } on both . and ./*, both targets present in dist
npm pack that dist, install the tarball into a fresh project (TypeScript 5.9.3, module: preserve, moduleResolution: bundler), run tsc exit 2 (TS7016) → exit 0

yarn test is 89 passed / 1 failed. The one failure,
packages/graphql-modules/tests/memory-leak.spec.ts, reproduces on an
unmodified checkout of master, so it is not from this change.

Note on ordering

The usual guidance is that types must be listed first. For this package it
turns out not to be what binds — I checked both orderings against the published
tarball, and types first and types last both resolve; only its absence
fails. This PR lists it first anyway, as the convention.

`packages/graphql-modules/package.json` has declared `exports["."].types` and
`exports["./*"].types` for years, but they never reach npm. `bob build` rewrites
the exports map for the published `dist/` directory via its `rewriteExports`,
which rebuilds every entry keeping only `require` and `import`:

    newValue = {
      require: newValue.require?.replace(`${distDir}/`, ""),
      import:  newValue.import?.replace(`${distDir}/`, ""),
    };

Anything else in the condition object -- `types` included -- is dropped. Every
published tarball up to and including 3.1.2 therefore has an exports map with no
`types` condition, so the declarations are unreachable under `moduleResolution`
`bundler`, `node16` and `nodenext`:

    error TS7016: Could not find a declaration file for module 'graphql-modules'.
    '.../graphql-modules/index.mjs' implicitly has an 'any' type.
      There are types at '.../graphql-modules/index.d.ts', but this result could
      not be resolved when respecting package.json "exports".

Only the legacy `node10` resolver worked, because it reads the top-level
`typings` field and ignores `exports` entirely. That is the problem reported in
 graphql-hive#2480, which was closed without it going away, and it still reproduces on the
latest release.

The conditions are reinstated in `scripts/fix-async-context.js`, which already
post-processes `dist/package.json` for exactly this class of loss -- its existing
comment reads "because bob removes the imports field". Doing it there needs no
bob upgrade, which matters because the v7 bump was tried in 0cc7337 and reverted
in 31be509. The restore is driven from the source package.json, so it is a no-op
if `types` is ever dropped there, and it leaves the condition first.

Verified end to end rather than by inspection:

  - before: `yarn build` then reading dist/package.json -> no `types` condition
  - after:  `yarn build` -> `{ types, require, import }` on both `.` and `./*`,
            both targets present in dist
  - packed the built dist with `npm pack`, installed the tarball into a fresh
    project on TypeScript 5.9.3 with `module: preserve` +
    `moduleResolution: bundler`, and `tsc` goes from exit 2 (TS7016) to exit 0

`yarn test` is 89 passed / 1 failed, and that one failure
(tests/memory-leak.spec.ts) reproduces on an unmodified checkout of master, so
it is not from this change.
@dotansimha
dotansimha requested a review from enisdenjo October 13, 2026 17:47

This branch has not been deployed

No deployments
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