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

@nx/js:tsc produces CommonJS instead of ESM for module: Node16 #18801

Open
2 of 4 tasks
jan-molak opened this issue Aug 23, 2023 · 6 comments
Open
2 of 4 tasks

@nx/js:tsc produces CommonJS instead of ESM for module: Node16 #18801

jan-molak opened this issue Aug 23, 2023 · 6 comments

Comments

@jan-molak
Copy link

Current Behavior

Introduced in NX 16.7.0, the function determineModuleFormatFromTsConfig looks at tsconfig.json to determine whether @nx/js:tsc should produce a CommonJS or an ESM module:

export function determineModuleFormatFromTsConfig(
  absolutePathToTsConfig: string
): 'cjs' | 'esm' {
  const tsConfig = readTsConfig(absolutePathToTsConfig);
  if (
    tsConfig.options.module === ts.ModuleKind.ES2015 ||
    tsConfig.options.module === ts.ModuleKind.ES2020 ||
    tsConfig.options.module === ts.ModuleKind.ES2022 ||
    tsConfig.options.module === ts.ModuleKind.ESNext
  ) {
    return 'esm';
  } else {
    return 'cjs';
  }
}

However, the function doesn't consider "module": "Node16", so if a TypeScript module happens to use it, @nx/js:tsc produces a package.json with all the export fields indicating an esm module, but with "type": "commonjs".

Expected Behavior

"module": "Node16" setting should result in @ns/js:tsc producing an esm module.

GitHub Repo

No response

Steps to Reproduce

Create a TypeScript library module and configure its tsconfig.json as follows:

{
  "extends": "../../tsconfig.base.json",
  "compilerOptions": {
    "module": "Node16",
    "moduleResolution": "Node16",
    "allowSyntheticDefaultImports": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "noImplicitOverride": true,
    "noPropertyAccessFromIndexSignature": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "types": ["vitest"],
    "jsx": "react-jsx"
  },
  "files": [],
  "include": [],
  "references": [
    {
      "path": "./tsconfig.lib.json"
    },
    {
      "path": "./tsconfig.spec.json"
    }
  ]
}

Run build and notice resulting package.json similar to the below:

{
  "name": "@my-namespace/my-module",
  "version": "1.0.0",
  "type": "commonjs",
  "types": "./src/index.d.ts",
  "exports": {
    ".": "./src/index.js",
    "./package.json": "./package.json"
  },
  "engines": {
    "node": "^16.13 || ^18.12 || ^20"
  },
  "module": "./src/index.js",
  "main": "./src/index.js"
}

Since the module is incorrectly marked as commonjs, consumers can't import its exports.

Nx Report

>  NX  Falling back to ts-node for local typescript execution. This may be a little slower.
  - To fix this, ensure @swc-node/register and @swc/core have been installed

 >  NX   Report complete - copy this into the issue template

   Node   : 18.16.0
   OS     : darwin-arm64
   npm    : 9.5.1
   
   nx                 : 16.7.4
   @nx/js             : 16.7.4
   @nx/jest           : 16.7.4
   @nx/linter         : 16.7.4
   @nx/workspace      : 16.7.4
   @nx/devkit         : 16.7.4
   @nx/eslint-plugin  : 16.7.4
   @nx/plugin         : 16.7.4
   @nrwl/tao          : 16.7.4
   @nx/vite           : 16.7.4
   typescript         : 5.1.6
   ---------------------------------------
   Community plugins:
   @theunderscorer/nx-semantic-release : 2.5.0
   ---------------------------------------
   Local workspace plugins:
         @serenity-js-x/nx-playwright-ct

Failure Logs

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

No response

@ajwootto
Copy link
Contributor

ajwootto commented Nov 2, 2023

From my understanding, the node16 setting for module doesn't imply either ESM or Commonjs, it bases its behaviour on what the module type is understood to be by looking at the package.json "type" field and other things (mimicking what Node does)

See the relevant explanation here
https://www.typescriptlang.org/docs/handbook/modules/reference.html#node16-nodenext

For that reason I don't think it's possible to determine the "correct" type of module to output based solely on that tsconfig setting. It would actually need to look at whether your source package.json "type" is already set to "module"

Because of this I don't really think determining the type of module from the "module" tsconfig setting actually makes sense in general, it should probably be forced to be specified in the package.json file of the source..

Copy link

github-actions bot commented May 1, 2024

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs.
If we missed this issue please reply to keep it active.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label May 1, 2024
@wootencl
Copy link

wootencl commented May 1, 2024

Not stale.

@ajwootto
Copy link
Contributor

ajwootto commented May 1, 2024

Also mentioning this related issue since I think basically Nx's whole handling of the Node16 setting in Typescript isn't working
#18324

Copy link

This issue has been automatically marked as stale because it hasn't had any activity for 6 months.
Many things may have changed within this time. The issue may have already been fixed or it may not be relevant anymore.
If at this point, this is still an issue, please respond with updated information.
It will be closed in 21 days if no further activity occurs.
Thanks for being a part of the Nx community! 🙏

@github-actions github-actions bot added the stale label Nov 12, 2024
@stephenwade
Copy link
Contributor

Not stale.

@github-actions github-actions bot removed the stale label Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants