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

Bundling for Node.js fails with @rollup/plugin-node-resolve>=11.0.0 #544

Closed
josias-r opened this issue Dec 7, 2020 · 6 comments
Closed

Comments

@josias-r
Copy link

josias-r commented Dec 7, 2020

Describe the bug

Your source code checks for something like this: typeof crypto !== 'undefined' && crypto.getRandomValues && crypto.getRandomValues.bind(crypto) || typeof msCrypto !== 'undefined' && typeof msCrypto.getRandomValues === 'function' && msCrypto.getRandomValues.bind(msCrypto); to find out whether crypto is available.

I'm not sure exactly where it goes wrong, but this package inside a compiled js file with rollup, will never import crypto and only check the globals like mentioned above.

How to reproduce

With rollup and the following rollup.config, compile a file that uses "uuid":

import typescript from "@rollup/plugin-typescript";
import commonjs from "@rollup/plugin-commonjs";
import nodeResolve from "@rollup/plugin-node-resolve";
import json from "@rollup/plugin-json";

export default {
  input: ["myfile.ts"],
  output: {
    dir: "dist",
    format: "cjs",
  },
  plugins: [
    commonjs(),
    nodeResolve(),
    json(),
    typescript(),
  ],
};

myfile.ts

import { v4 as uuidv4 } from "uuid";
console.log(uuidv4());

Expected behavior

The example should have something like const crypto = require("crypto") somewhere in the compiled code. Currently, it only checks whether it is available globally, but in my use case I'm not in a browser environment, but a nodejs environment, so it would need to get imported to work.

As a note: I can see from the source that rng.js seems to be set-up correctly, but rng-browser.js is doing the unsupported check in a node environment. So the question really is, why I'm getting the browser package instead of the node one. Can I control this? (rollup is already set-up for node, aka cjs)

@ctavan
Copy link
Member

ctavan commented Dec 7, 2020

@josias-r seems like you discovered a bug with the most recent version of @rollup/plugin-node-resolve, see rollup/plugins#695

I suggest you watch that issue for updates. Until then you can downgrade to @rollup/plugin-node-resolve@10.0.0 and should be good to go.

@lukastaegert
Copy link

lukastaegert commented Dec 8, 2020

You need to specify nodeResolve({exportConditions: ["node"]}) as node is not one of the default conditions, see the documentation: https://github.com/rollup/plugins/tree/master/packages/node-resolve#exportconditions

@josias-r
Copy link
Author

josias-r commented Dec 8, 2020

@ctavan @lukastaegert Thank you guys, this resolved my issue! The docs you linked don't really show node as an option, so I would have never thought of that.

Why exactly this was not the behaviour in an old version/what exactly is going on I still don't fully understand, but I'll watch the other issue for that sake 😄

@ctavan
Copy link
Member

ctavan commented Dec 8, 2020

Keeping this open until rollup/plugins#695 has settled.

@ctavan ctavan reopened this Dec 8, 2020
@ctavan ctavan changed the title Crypto is undefined (crypto.getRandomValues) Bundling for Node.js fails with @rollup/plugin-node-resolve>=11.0.0 Dec 8, 2020
@thegreatsunra
Copy link

thegreatsunra commented Jan 7, 2021

I got bit by this issue as well, when bundling uuid for Node.js with @rollup/plugin-node-resolve>=11.0.0.

You need to specify nodeResolve({exportConditions: ["node"]}) as node is not one of the default conditions, see the documentation: https://github.com/rollup/plugins/tree/master/packages/node-resolve#exportconditions

"node" isn't listed as a value in those docs (at least at the time of writing; sounds like the docs will be updated soon) but I can confirm this fix worked for me, so thank you!

@ctavan
Copy link
Member

ctavan commented May 15, 2021

Great to see that this has been resolved. I've proposed a PR to clarify the solution in the rollup plugin docs: rollup/plugins#884

I'll already close this one since it is not a problem with the uuid library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants