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: main field in package.json should correspond to cjs artifacts #5756
base: v1.x
Are you sure you want to change the base?
Conversation
When axios#4787 was implemented, the project was switched to `"type": "module"` and "./index.js" became an esm file instead of commonjs, however, the "main" entry in package.json still points to "index.js". As a result, consumers using this field may get unexpected behavior since the main field is supposed to be commonjs if the entry is provided. Many consumers won't run into this as a practical problem (for example when just doing `const axios = require('axios').default` from inside of a cjs file in node) because the "exports" map takes precedence over the main/module fields, but tools that don't parse the object map when resolving still run into problems here. The fix for this is to just point the "main" entry-point to the commonjs artifacts located at "./dist/node/index.cjs". I also added a module entrypoint to improve compatability for the cases where the export map is not used (webpack 4 for example) since that would likely be reading the cjs "main" entrypoint now that main has switched back to cjs.
This PR has been sitting for a few months but the underlying issue has been causing me all sorts of headaches as we need to keep adding the following to our jest.config.js to work around these issues which has been confusing for our end users (since they aren't importing axios directly and its a transitive dependency): moduleNameMapper: {
'^axios$': require.resolve('axios'),
}, @DigitalBrainJS I don't know who the current project owners are, but is this something you are able to review and release? |
Why do you think this should not be so? Axios is ESM module and according to the documentation, the main field must point to an ESM module if the package has "type: module". "Module" field is not in npm spec and it is just a proprietary field for some bundlers. According to the npm documentation, backward compatibility with commonjs for an ESM package should be handled by conditional export. |
I haven't found that in the node documentation specifically that implies the main field must be the es-module the package type is module. I could only find documentation that suggested that it could point to cjs or esm (in which case the package would not be consumable from a For example, there is the following quote:
While that says the can point to the same module, their example doesn't use conditional exports or specify https://nodejs.org/api/packages.html
So I tested this in node 12.0 and as you say, the conditional export isn't used and so the following code fails to work in older versions of node where the main field is used instead of the export map: const {default: axios} = require('axios');
axios.get('http://www.google.com').then(result => console.log(result.data)); Fails with:
But node versions 12.0-7 were never LTS versions, and they are certainly out of date, so maybe we don't need the main field at all if your intention is to not support special cases as a result of obsolescence. I guess my question is, if we are relying on the exports map for this functionality to work correctly in node, what is the intended use-case for setting the "main" entrypoint to point to the esm version of the package? The field will never be used in current versions of node and breaks in old versions, so maybe it is because that is what would be used by bundlers that don't support the exports map? If that's the case, those same bundlers support the proprietary "module" and "browsers" entry-points. There's also a ton of other entries already in this I can maybe see the case for using: "browser": {
".": "./index.js", instead of |
@DigitalBrainJS I have worked around this issue on my end by removing my dependency on |
When #4787 was implemented, the project was switched to
"type": "module"
and "./index.js" became an esm file instead of commonjs, however, the "main" entry in package.json still points to "index.js". As a result, consumers using this field may get unexpected behavior since the main field is supposed to be commonjs if the entry is provided.Many consumers won't run into this as a practical problem (for example when just doing
const axios = require('axios').default
from inside of a cjs file in node) because the "exports" map takes precedence over the main/module fields, but tools that don't parse the exports map when resolving still run into problems here. As an example, I encountered this problem when using the resolve package as part of a workflow for a jest config.The fix for this is to just point the "main" entry-point to the commonjs artifacts located at "./dist/node/axios.cjs".
I also added a module entrypoint to improve compatibility for the cases where the export map is not used (webpack 4 for example) since that would likely be reading the cjs "main" entrypoint now that main has switched back to cjs.