Skip to content
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

fix: change main entry to CJS dist #6348

Open
wants to merge 1 commit into
base: v1.x
Choose a base branch
from

Conversation

lukekarrys
Copy link

@lukekarrys lukekarrys commented Apr 10, 2024

ESM aware tools will only look in exports (and not main) when resolving a package. Therefore the main entry point should always be a CJS file.

I believe this is just an oversight in the v1.x branch since the last commit touching package.json#main was 10 years ago.

This change will only affect older runtimes like node@<=10 as well as tools like resolve. I'm not sure if those Node versions are officially supported, but this change should be ignored by all new runtime versions and bundlers.

I found this due to a mocking library our tests were using that would fallback to the resolve package. In that case Node's require.resolve would end up pointing to a different path than resolve:

❯ node --version
v20.12.1
❯ node -e 'console.log(require.resolve("axios"))'
/Users/lukekarrys/Desktop/hmmmm/node_modules/axios/dist/node/axios.cjs
❯ node -e 'console.log(require("resolve").sync("axios"))'
/Users/lukekarrys/Desktop/hmmmm/node_modules/axios/index.js

Similar things happen in Node 10 that cause the require to throw:

❯ node --version
v10.24.1
❯ node -e 'console.log(require("axios/dist/node/axios.cjs").VERSION)'
1.6.8
❯ node -e 'console.log(require("axios").VERSION)'
/Users/lukekarrys/Desktop/wtf/node_modules/axios/index.js:1
import axios from './lib/axios.js';
       ^^^^^

SyntaxError: Unexpected identifier
    at Module._compile (internal/modules/cjs/loader.js:723:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at [eval]:1:13
    at Script.runInThisContext (vm.js:122:20)
    at Object.runInThisContext (vm.js:329:38)

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.

None yet

3 participants