nx icon indicating copy to clipboard operation
nx copied to clipboard

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

Open jan-molak opened this issue 1 year ago • 4 comments

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

  • [X] macOS
  • [X] Linux
  • [ ] Windows
  • [ ] Other (Please specify)

Additional Information

No response

jan-molak avatar Aug 23 '23 19:08 jan-molak

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..

ajwootto avatar Nov 02 '23 17:11 ajwootto

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[bot] avatar May 01 '24 00:05 github-actions[bot]

Not stale.

wootencl avatar May 01 '24 00:05 wootencl

Also mentioning this related issue since I think basically Nx's whole handling of the Node16 setting in Typescript isn't working https://github.com/nrwl/nx/issues/18324

ajwootto avatar May 01 '24 17:05 ajwootto