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

package.json should use conditional exports, else esm won't work inexplicitly #310

Open
loynoir opened this issue Feb 14, 2023 · 1 comment

Comments

@loynoir
Copy link

loynoir commented Feb 14, 2023

Actual

inexplicitly

import {compare} from 'fast-json-patch'
console.log({compare})
import {compare} from 'fast-json-patch'
        ^^^^^^^
SyntaxError: Named export 'compare' not found. The requested module 'fast-json-patch' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'fast-json-patch';
const {compare} = pkg;

explicitly

import {compare} from 'fast-json-patch/index.mjs'
console.log({compare})
Could not find a declaration file for module 'fast-json-patch/index.mjs'. 
'/path/to/node_modules/fast-json-patch/index.mjs' implicitly has an 'any' type.
If the 'fast-json-patch' package actually exposes this module, 
try adding a new declaration (.d.ts) file containing `declare module 'fast-json-patch/index.mjs'
{ compare: [Function: compare] }

Expected

import {compare} from 'fast-json-patch'
console.log({compare})
{ compare: [Function: compare] }
@loynoir loynoir changed the title package.json should use conditional exports, else esm won't work package.json should use conditional exports, else esm won't work inexplicitly Feb 14, 2023
@anonghuser
Copy link

+1

To be more specific on what is needed, the following snippet should be added to package.json:

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

This is the official NodeJS-supported equivalent of the "module" field which is an unofficial extension of package.json by tools such as webpack. It is documented here.

At least index.mjs should remain exported as a subpath, since it was documented in the README. Even if you remove it from the README, I think it should remain exported for backwards compatibility at least until a major version change. If you are aware people use other subpaths, i.e. require('fast-json-patch/commonjs/core.js'), either list them explicitly too or include a "./*":"./*" mapping instead of the explicitly listed index.mjs. If you want the .js extension to be optional as it is in the default require behavior so that require('fast-json-patch/commonjs/core') works, something like this could work:

        "./*.mjs": "./*.mjs",
        "./*.js": "./*.js",
        "./*": "./*.js"

There is also a provision for specifying the location of the type definition files or even full typescript sources in the new syntax instead of using undocumented fields at the root level, but I don't think typescript tools actually support it yet so it's a point for another time.

As for the Could not find a declaration file for module error mentioned in the issue when someone references a .mjs file, the .d.ts could be copied to .d.mts to resolve it. Perhaps it is pointless to do for index.mjs since there will be no need to use it after this update, but if you expect people to be referencing the modules/*.mjs files then rename their type definition files to .d.mts. This is somewhat tangential to the main issue though.

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

No branches or pull requests

2 participants