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

Support node16 module resolution for CJS projects #33

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

myftija
Copy link
Contributor

@myftija myftija commented Oct 24, 2023

Background

CJS projects with moduleResolution set to node16 currently cannot consume modules published using dual-publish. The root cause is in the package.json exports. See this issue for more details: ai/nanoid#422.

Changes in this PR

  • If there is a .d.ts types file specified in package.json, it is cloned into a .d.cts types file. Typescript can correctly interpret this as a CJS module.
  • The require and import conditions in exports now each contain types and default conditions. This way we can use separate type files for ESM and CJS.

The generated exports field now looks like this:

"exports": {
  ".": {
    "browser": "./index.browser.js",
    "require": {
      "types": "./index.d.cts",
      "default": "./index.cjs"
    },
    "import": {
      "types": "./index.d.ts",
      "default": "./index.js"
    },
    "default": "./index.js"
  }
}

instead of:

"exports": {
  ".": {
    "types": "./index.d.ts",
    "browser": "./index.browser.js",
    "require": "./index.cjs",
    "import": "./index.js",
    "default": "./index.js"
  }
}

Tested the changes with https://github.com/ai/nanoid/tree/v3. I could successfully consume the resulting module from a CJS project with node16 module resolution.

}

if (!pkg.types.endsWith('.d.ts')) {
throw error('The types file extension must be .d.ts.')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

would this be a reasonable assumption?

@myftija
Copy link
Contributor Author

myftija commented Oct 25, 2023

@ai I noticed that the CI is failing with this error from pnpm: ERR_PNPM_FROZEN_LOCKFILE_WITH_OUTDATED_LOCKFILE. Maybe due to the pnpm version not being pinned in the CI. Would you like me to update the lockfile and add it to this PR? Or are you planning to take care of it in a separate PR?

@ai
Copy link
Owner

ai commented Oct 25, 2023

Don't worry, I will fix it. I will take it a few days later.

@ai ai merged commit 6f35a4f into ai:main Nov 3, 2023
0 of 2 checks passed
@ai
Copy link
Owner

ai commented Nov 3, 2023

Thanks. Released in 4.0. I will release nanoid 3.x fix soon.

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.

2 participants