Skip to content

Add exports condition and remaining tweaks#339

Open
jerelmiller wants to merge 5 commits intofb/0.10.0from
jerel/tweaks
Open

Add exports condition and remaining tweaks#339
jerelmiller wants to merge 5 commits intofb/0.10.0from
jerel/tweaks

Conversation

@jerelmiller
Copy link
Member

This PR does the remaining work from my suggestions in #331 and adds an exports field to package.json to work with modern bundlers.

"moduleResolution": "node",
"lib": ["ES2023", "dom"],
"module": "NodeNext",
"moduleResolution": "NodeNext",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated these to match Apollo Client 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jerelmiller jerelmiller mentioned this pull request Dec 12, 2025
1 task
Copy link
Collaborator

@fbartho fbartho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks the help @jerelmiller !

"moduleResolution": "node",
"lib": ["ES2023", "dom"],
"module": "NodeNext",
"moduleResolution": "NodeNext",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@phryneas
Copy link
Member

From the npm folder after running the deploy command locally (without actually publishing):

% npx  @arethetypeswrong/cli                                                                                                  jerel/tweaks
Need to install the following packages:
@arethetypeswrong/cli@0.18.2
Ok to proceed? (y) 

Run `npm pack`? (Pass -P/--pack to skip) (Y/n) 

apollo-link-rest v0.10.0-rc.2

Build tools:
- typescript@5.x
- rollup@^4.53.2

⚠️ A require call resolved to an ESM JavaScript file, which is an error in Node and some bundlers. CommonJS consumers will need to use a dynamic import. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/CJSResolvesToESM.md

🐛 Import resolved to types through a conditional package.json export, but only after failing to resolve through an earlier condition. This behavior is a TypeScript bug. It may misrepresent the runtime behavior of this import and should not be relied upon. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/FallbackCondition.md

🚭 Syntax detected in the module is incompatible with the module kind according to the package.json or file extension. This is an error in Node and may cause problems in some bundlers. https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/main/docs/problems/UnexpectedModuleSyntax.md


┌───────────────────┬──────────────────────────────┐
│                   │ "apollo-link-rest"           │
├───────────────────┼──────────────────────────────┤
│ node10            │ 🟢                           │
├───────────────────┼──────────────────────────────┤
│ node16 (from CJS) │ ⚠️ ESM (dynamic import only) │
│                   │ 🐛 Used fallback condition   │
│                   │ 🚭 Unexpected module syntax  │
├───────────────────┼──────────────────────────────┤
│ node16 (from ESM) │ 🟢 (ESM)                     │
├───────────────────┼──────────────────────────────┤
│ bundler           │ 🟢                           │
└───────────────────┴──────────────────────────────┘

package.typings = 'index.d.ts'; \
package.exports['.'].module = './index.js'; \
package.exports['.']['module-sync'] = './index.js'; \
package.exports['.'].require = './bundle.umd.js'; \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem here is the .umd.js extension.

image

As weird as it sounds, the name of this file would need to be

-./bundle.umd.js
+./bundle.umd.cjs

for this to work.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or we skip the type: module, and all other files are renamed to mjs.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing @phryneas! My first reaction is to go with the .cjs extension, because I believe that tagging this with ”type”: “module” is considered best practice & it sounds like an easy fix, but I don’t actually know for sure.

What do you think @jerelmiller ?

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.

3 participants