Skip to content

Support pkg.exports.node field when building for Node.js #695

New issue
New issue
Closed
Closed
Support pkg.exports.node field when building for Node.js#695
Labels
c⁴ ⋅ need more infot¹ 🐞 bug🔌 plugin-node-resolve

Description

@ctavan
ctavan
opened on Dec 7, 2020
  • Rollup Plugin Name: @rollup/plugin-node-resolve
  • Rollup Plugin Version: 11.0.0
  • Rollup Version: 2.34.2
  • Operating System (or Browser): any
  • Node Version: 15.x
  • Link to reproduction: https://repl.it/@ctavan/rollup-plugin-repro

Expected Behavior

@rollup/plugin-node-resolve@11.0.0 added support for pkg.exports entrypoints through #540.

However it seems like the node keyword is currently not respected which causes trouble when bundling for Node.js environments.

The specific pkg.exports definition that doesn't work as expected is https://github.com/uuidjs/uuid/blob/334ef62c330d92f8ca376d09087e8ee9abc5cc12/package.json#L21-L31:

{
  "exports": {
    ".": {
      "node": {
        "module": "./dist/esm-node/index.js",
        "require": "./dist/index.js",
        "import": "./wrapper.mjs"
      },
      "default": "./dist/esm-browser/index.js"
    },
    "./package.json": "./package.json"
  }
}

Since apparently the node keyword is ignored, rollup picks up the default key, which in this case is a browser build.

Node.js supports the node keyword and this is supported in webpack@5 as well

It has been reported as a bug to uuidjs/uuid#544

Actual Behavior

When browser: false, respect the node keyword, just like Node.js and webpack do.

Additional Information

/cc @guybedford @LarsDenBakker @TrySound @lukastaegert since you were involved in earlier discussions around pkg.exports and uuid as a widely used canary, see rollup/rollup#3514 & webpack/webpack#11014 😉

Edit: Here's the reference where @guybedford proposed the pattern used in uuid's pkg.exports field: uuidjs/uuid#468 (comment)

Activity

  1. shellscape commented on Dec 8, 2020

    @shellscape
    shellscape
    on Dec 8, 2020
    Collaborator

    Have you seen #693?

  2. lukastaegert commented on Dec 8, 2020

    @lukastaegert
    lukastaegert
    on Dec 8, 2020
    Member

    In the examples mentioned, I do not see the node condition specified explicitly. If you read the plugin documentation, you will see that the only conditions set by default are ['default', 'module', 'import']. I am not sure whether the absence of browser should automatically imply node, especially since browser is to my knowledge not yet integrated into package.exports resolution anyway (you would still need to specify the browser condition explicitly). That being said, there is definitely room for improvement, at the very least on documentation level but also to make things clearer for the user with less screws to turn.

  3. ctavan commented on Dec 8, 2020

    @ctavan
    ctavan
    on Dec 8, 2020
    ContributorAuthor

    @shellscape thanks for the pointer. I must have missed that issue when searching before raising this report. After reading through it I'm still unsure if it really solves this issue out of the box.

    @lukastaegert I was referring to

    a) The node.js documentation at https://nodejs.org/api/packages.html#packages_conditional_exports which states:

    Node.js supports the following conditions out of the box:

    • "import" - matches when the package is loaded via import or import(), or via any top-level import or resolve operation by the ECMAScript module loader. Applies regardless of the module format of the target file. Always mutually exclusive with "require".
    • "require" - matches when the package is loaded via require(). The referenced file should be loadable with require() although the condition matches regardless of the module format of the target file. Expected formats include CommonJS, JSON, and native addons but not ES modules as require() doesn't support them. Always mutually exclusive with "import".
    • "node" - matches for any Node.js environment. Can be a CommonJS or ES module file. This condition should always come after "import" or "require".
    • "default" - the generic fallback that always matches. Can be a CommonJS or ES module file. This condition should always come last.

    b) The Webpack documentation at https://webpack.js.org/guides/package-exports/#target-environment which states:

    The following conditions are set depending on the target environment:

    Condition Description Supported by
    browser Code will run in a browser. webpack
    electron Code will run in electron. webpack
    worker Code will run in a (Web)Worker. webpack
    worklet Code will run in a Worklet.  
    node Code will run in Node.js. webpack, Node.js
    deno Code will run in Deno.  
    react-native Code will run in react-native.

    Now when looking at https://rollupjs.org/guide/en/#quick-start the example "For Node.js"

    rollup main.js --file bundle.js --format cjs

    somewhat suggests that the resulting bundle is supposed to run in Node.js. That's why I made the assumption that when bundling for CJS I would automatically get the Node.js-style package.exports resolution. But maybe I'm misinterpreting things here a bit.

    I do agree that browser: false can probably not automatically mean node: true…

    So maybe now that package.exports support arrived in the node-resolve plugin it's now also time for a target: option for the node-resolve plugin, similar to what webpack does? Otherwise I see a lot of potential for confusion around this topic.

  4. lukastaegert commented on Dec 8, 2020

    @lukastaegert
    lukastaegert
    on Dec 8, 2020
    Member

    Definitely, and no matter how we go from here, there should be clear examples in the documentation that show-case the main use-cases (i.e. bundling for Node and bundling for the web).

  5. shellscape commented on Feb 1, 2021

    @shellscape
    shellscape
    on Feb 1, 2021
    Collaborator

    @ctavan can you double check this against the latest version?

  6. shellscape
    added
    c⁴ ⋅ need more info
    t¹ 🐞 bug
    🔌 plugin-node-resolve
    on Feb 1, 2021
  7. stale
    added
    x⁷ ⋅ stale
    on Apr 2, 2021

  8. ctavan commented on Apr 2, 2021

    @ctavan
    ctavan
    on Apr 2, 2021
    ContributorAuthor

    @shellscape I've updated the Replit to use @rollup/plugin-node-resolve@11.2.1 but the issue is still there.

    If I try to execute the resulting bundle with Node.js I get:

    $ node output/bundle.js 
\/home/runner/rollup-plugin-repro/output/bundle.js:12
    throw new Error('crypto.getRandomValues() not supported. See https://github.com/uuidjs/uuid#getrandomvalues-not-supported');
    ^

    Rollup still seems to pick up the browser build instead of the node build.

  9. stale
    removed
    x⁷ ⋅ stale
    on Apr 2, 2021

  10. guybedford commented on Apr 30, 2021

    @guybedford
    guybedford
    on Apr 30, 2021
    Contributor

    This can now be configured when using the exportsConditions: ['node'] option of plugin-node-resolve. If there are any further issues here please do let us know though.

  11. guybedford
    closed this as completedon Apr 30, 2021

  12. ctavan commented on May 15, 2021

    @ctavan
    ctavan
    on May 15, 2021
    ContributorAuthor

    I have configured exportsConditions: ['node'] instead of browser: false in https://replit.com/@ctavan/rollup-plugin-repro#rollup.config.js but rollup still seems to pick up the browser bundle from uuid.

    What am I doing wrong?

    FTR: I realized it must read exportConditions: ['node'] instead of exportsConditions: ['node'] (there was an "s" too much). It now works! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    c⁴ ⋅ need more infot¹ 🐞 bug🔌 plugin-node-resolve

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions