-
Notifications
You must be signed in to change notification settings - Fork 2.1k
enable conditional exports #4448
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The secret sauce is here: https://github.com/yaacovCR/graphql-js/blob/dual/resources/build-npm.ts#L204-L221 function buildExports(filepath: string): ConditionalExports {
const { dir, name } = path.parse(filepath);
const base = `./${path.join(dir, name)}`;
return {
types: {
module: `${base}.d.mts`,
'module-sync': `${base}.d.mts`,
node: `${base}.d.ts`,
require: `${base}.d.ts`,
default: `${base}.d.mts`,
},
module: `${base}.mjs`,
'module-sync': `${base}.mjs`,
node: `${base}.js`,
require: `${base}.js`,
default: `${base}.mjs`,
};
} Based on: UPDATED: the |
This solves node -- but what about bun and deno: Bun seems to support: Deno seems to support: We would probably need to add custom conditions for bun and deno. |
With further testing, it seems that require(esm) on deno, at least how I am using it, does not honor the deno condition? So we can set deno to cjs or just drop it, because deno does consistently honor the node condition…. |
60ad23e
to
03d4440
Compare
update/summary: bun, module, module-sync => ESM deno and node (in versions that do not support module-sync) point to CJS even when using import so as to avoid the dual-package hazard. the require condition points to CJS just because our default is otherwise now ESM. |
this commits adds support for conditional exports bun, module, module-sync => ESM deno, node and require => CJS default => ESM. deno and node (in versions that do not support module-sync) point to CJS even when using import so as to avoid the dual-package hazard. the require condition points to CJS just because our default is otherwise now ESM.
feedback off-site from @phryneas: we can drop the types condition, this is mentioned in the Node.JS documentation, but it seems like all relevant TS versions actually support automatic checking of sibling .d.ts/.d.mts/.d.cts files, as done deno, added deno check. We can add this back in if for somehow we have this wrong. See: apollographql/apollo-client#12628 See also: microsoft/TypeScript#50762 where the types condition can be erroneously put at the end, and is described as useful only for complicated setups without sibling files. |
depends on #4446
before:
i.e. importing from esm gives you cjs
after:
we have conditional exports, and it looks like nothing has changed.....
BUT:
Any version of Node honoring the module-sync condition i.e. where require(esm) is enabled will load ESM both when imported and required, i.e.
^20.19.0 || ^22.12.0 || >=23.0.0
, i.e. latest versions of all current Node.JS releases.We have been delaying this for some time secondary to dual-package-hazard concerns, but this looks like we can release ESM to the broader community via package exports while retaining whatever legacy CJS build we might need for those environments that need it => looking at you Jest.
We can look into whether we want to drop cjs support entirely as a follow-on PR => see #4437