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/rollup built library errors when using new .cjs.mjs file #20009

Closed
1 of 4 tasks
thomassimko opened this issue Nov 2, 2023 · 18 comments · Fixed by #21473
Closed
1 of 4 tasks

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

thomassimko opened this issue Nov 2, 2023 · 18 comments · Fixed by #21473
Assignees
Labels
outdated scope: bundlers Issues related to webpack, rollup type: bug

Comments

@thomassimko
Copy link

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

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

Additional Information

No response

@jaysoo jaysoo added the scope: bundlers Issues related to webpack, rollup label Nov 2, 2023
@jaysoo jaysoo self-assigned this Nov 2, 2023
@jaysoo
Copy link
Member

jaysoo commented Nov 9, 2023

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

@thomassimko
Copy link
Author

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.

@alexgavrusev
Copy link

alexgavrusev commented Nov 17, 2023

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?

@davidlinhares
Copy link

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)

@kajurek
Copy link

kajurek commented Dec 8, 2023

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

Copy link

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 Dec 23, 2023
@thomassimko
Copy link
Author

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

@github-actions github-actions bot removed the stale label Dec 24, 2023
Copy link

github-actions bot commented Jan 7, 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 Jan 7, 2024
@alexgavrusev
Copy link

Not stale.

@github-actions github-actions bot removed the stale label Jan 8, 2024
@jclarkcisco
Copy link

@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
Copy link

@kajurek - your repo link above is not available.

@thomassimko
Copy link
Author

thomassimko commented Jan 12, 2024

@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
Copy link
Member

jaysoo commented Jan 16, 2024

Thanks for the repro, we'll take a look

@jclarkcisco
Copy link

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

@xiongemi
Copy link
Collaborator

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

@alexgavrusev
Copy link

alexgavrusev commented Jan 19, 2024

@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

@xiongemi
Copy link
Collaborator

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.

Copy link

github-actions bot commented Mar 4, 2024

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 github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: bundlers Issues related to webpack, rollup type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants