nx icon indicating copy to clipboard operation
nx copied to clipboard

@nx/rollup built library errors when using new .cjs.mjs file

Open thomassimko opened this issue 1 year ago • 17 comments

Current Behavior

After upgrading from NX v16 to NX v17, the build outputs for our React packages changed. Notably the exports section changed.

// NX v16

"exports": {
  "./index.css": "./index.css",
  ".": {
    "types": "./src/index.d.ts",
    "import": "./index.js", // <-- this one
    "require": "./index.cjs"
  }
},

// NX v17

"exports": {
  "./index.css": "./index.esm.css",
  ".": {
    "module": "./index.esm.js",
    "import": "./index.cjs.mjs", // <-- this one
    "default": "./index.cjs.js"
  },
  "./package.json": "./package.json"
},

Now on a consuming application that is using NextJS and importing via import { MODULE_NAME } from "PACKAGE", we are receiving errors that did not exist before.

export { _default as default } from './index.cjs.default.js';
         ^^^^^^^^^^^^^^^^^^^
SyntaxError: The requested module './index.cjs.default.js' does not provide an export named '_default'

I am not an expert in ESM or CJS, but it seems very odd to me to change how this is exported across the versions.

Expected Behavior

I would expect my consuming application to be able to consume the library without errors.

GitHub Repo

No response

Steps to Reproduce

Nx Report

>  NX   Report complete - copy this into the issue template

   Node   : 18.17.0
   OS     : darwin-x64
   npm    : 9.6.7
   
   nx (global)        : 16.5.3
   nx                 : 17.0.1
   lerna              : 6.5.1
   @nx/js             : 17.0.1
   @nx/jest           : 17.0.1
   @nx/linter         : 17.0.1
   @nx/eslint         : 17.0.1
   @nx/workspace      : 17.0.1
   @nx/cypress        : 17.0.1
   @nx/devkit         : 17.0.1
   @nx/eslint-plugin  : 17.0.1
   @nx/plugin         : 17.0.1
   @nx/react          : 17.0.1
   @nx/rollup         : 17.0.1
   @nx/storybook      : 17.0.1
   @nrwl/tao          : 17.0.1
   @nx/vite           : 17.0.1
   @nx/web            : 17.0.1
   @nx/webpack        : 17.0.1
   typescript         : 5.1.6
   ---------------------------------------
   Community plugins:
   nx-stylelint : 15.0.0
   ---------------------------------------
   Local workspace plugins:
         @magnetic-common-design-system/workspace-plugin

Failure Logs

export { _default as default } from './index.cjs.default.js';
         ^^^^^^^^^^^^^^^^^^^
SyntaxError: The requested module './index.cjs.default.js' does not provide an export named '_default'

Package Manager Version

No response

Operating System

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

Additional Information

No response

thomassimko avatar Nov 02 '23 17:11 thomassimko

@thomassimko Do you have a reprod for this? I have some questions:

  1. When do you see this error? During build, during runtime?
  2. Is the lib in the same monorepo?
  3. Are you using a custom server with Next.js?
  4. Are you using --turbo when running the dev server?
  5. Are you using pages or app router?

I tried to reproduce this issue but could not. The weird thing is that the serving and building should be reading the module export, not import. The import export would be used in Node if you were using a custom server.

jaysoo avatar Nov 09 '23 15:11 jaysoo

Let me try to reproduce in a stripped down project for you. It was also my understanding that NextJS should be pulling the module instead of the import export. To answer your questions:

  1. This error happens during build.
  2. The lib is not in the same monorepo. My team uses NX for a component library and another team is trying to use our library and running into the issue.
  3. No it does not use a custom server.
  4. No they do not use --turbo.
  5. It uses pages router.

thomassimko avatar Nov 09 '23 19:11 thomassimko

This happens because when there's only a default export, the CJS output exports it through module.exports, not exports['default'].

e.g. given

export class Foo {}

export default class Bar {}

the output is

// index.cjs.js

'use strict';

Object.defineProperty(exports, '__esModule', { value: true });

class Foo {
}
class Bar {
}

exports.Foo = Foo;
exports["default"] = Bar;

But given

export default class Bar {}

the output is

// index.cjs.js

'use strict';

class Bar {
}

module.exports = Bar;

To make the default export available in that case, the index.cjs.default.js can be adjusted as follows:

const e = require('./index.cjs.js')
exports._default = e.default ?? e;

@jaysoo would you like me to send a PR and/or provide a complete reproduction repo?

alexgavrusev avatar Nov 17 '23 15:11 alexgavrusev

I have created a sample NX repo where the component bundled has the output described by @alexgavrusev.

Repo: https://github.com/davidlinhares/nx-rollup-multi-config (it says multi config but this has a single rollup config)

davidlinhares avatar Nov 30 '23 14:11 davidlinhares

another sample repo, description in README file https://github.com/kajurek/bdb-monorepor-nx-issue

kajurek avatar Dec 08 '23 12:12 kajurek

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 Dec 23 '23 00:12 github-actions[bot]

Not stale. My coworkers are creating a new repo with this issue.

thomassimko avatar Dec 23 '23 00:12 thomassimko

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 Jan 07 '24 00:01 github-actions[bot]

Not stale.

alexgavrusev avatar Jan 07 '24 09:01 alexgavrusev

@jaysoo - any updates on this? We have several downstream products that are unable to utilize our product due to this. I'd like to know if there's a proposed timeline before we revert to a previous version of NX.

jclarkcisco avatar Jan 08 '24 21:01 jclarkcisco

@kajurek - your repo link above is not available.

jclarkcisco avatar Jan 08 '24 21:01 jclarkcisco

@jaysoo

I have created a new repository that has this issue: https://github.com/thomassimko/nx-default-build-error

You can see the issue by doing the following:

  1. npm i in the root directory -- this installs verdaccio as a local registry
  2. npm i in the nx directory -- this is an nx component monorepo that provides components to another external app
  3. npm run publish in the nx directory -- publishes both container and types packages to verdaccio
  4. npm i in the jest-project directory -- install deps for consuming app
  5. npm run test in the jest-project directory -- this runs the tests using vitest (I know I named the project poorly) and reports the error that we are seeing
 FAIL  src/App.test.tsx [ src/App.test.tsx ]
SyntaxError: The requested module './index.cjs.default.js' does not provide an export named '_default'

Please let me know if you have any other questions. This is a big issue for our team.

thomassimko avatar Jan 12 '24 00:01 thomassimko

Thanks for the repro, we'll take a look

jaysoo avatar Jan 16 '24 20:01 jaysoo

Thanks @jaysoo - please let us know if we can do anything else to help troubleshoot this.

jclarkcisco avatar Jan 17 '24 18:01 jclarkcisco

@jaysoo

I have created a new repository that has this issue: https://github.com/thomassimko/nx-default-build-error

You can see the issue by doing the following:

  1. npm i in the root directory -- this installs verdaccio as a local registry
  2. npm i in the nx directory -- this is an nx component monorepo that provides components to another external app
  3. npm run publish in the nx directory -- publishes both container and types packages to verdaccio
  4. npm i in the jest-project directory -- install deps for consuming app
  5. npm run test in the jest-project directory -- this runs the tests using vitest (I know I named the project poorly) and reports the error that we are seeing
 FAIL  src/App.test.tsx [ src/App.test.tsx ]
SyntaxError: The requested module './index.cjs.default.js' does not provide an export named '_default'

Please let me know if you have any other questions. This is a big issue for our team.

@jaysoo and I looked at the repo, it turns out the libraries have "type": "module" in the package.json, so that is why the viest is not resolving these libraries correctly. When adding "type": "module", it will ALWAYS resolve the package as an es module, whereas in the export field, as you mentioned, the import is in CJS:

".": {
    "module": "./index.esm.js",
    "import": "./index.cjs.mjs", // <-- this one
    "default": "./index.cjs.js"
  },

Quick fixes would be removing "type": "module" libraries' package.json:

  • nx/libs/types/package.json
  • nx/libs/container/package.json

So the consuming app will resolve these libraries correclty. i will submit a proper fix to detect when package.json has "type": "module" and only export esm format.

xiongemi avatar Jan 18 '24 20:01 xiongemi

@xiongemi after looking at the rules that publint defines, the is a mention that "type" should always be set:

Since Node.js v20.10.0, it introduces a new --experimental-default-type flag to flip the default module system from "CJS-as-default" to "ESM-as-default". If enabled, package.json without the "type" field will mean its descendant JS files to be interpreted as ESM instead of CJS, which may not work correctly.

While this only applies to files outside of node_modules, it's still recommended to set it up for future-proofing. And it also helps the --experimental-detect-module flag if enabled.

Hence, if you've not set the "type" field, you can explictly set it as "type": "commonjs" (default value), or migrate to "type": "module" and write in ESM completely if possible.

So that fix should probably also set "type": "commonjs", if it was not set to "module" by the user, but I'm not sure that ESM export will work in that case

alexgavrusev avatar Jan 19 '24 16:01 alexgavrusev

I have created a sample NX repo where the component bundled has the output described by @alexgavrusev.

Repo: https://github.com/davidlinhares/nx-rollup-multi-config (it says multi config but this has a single rollup config)

@davidlinhares i was looking at your repo, probably the same issue where in library libs/button/package.json, it has "type": "module", so that is why it could not resolve the library correctly.

xiongemi avatar Jan 19 '24 16:01 xiongemi

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

github-actions[bot] avatar Mar 04 '24 00:03 github-actions[bot]