chalk/chalk

When bundling with Rollup.js resulting bundle does not work under Node.js

jeroenmuller opened this issue · 2 comments

jeroenmuller commented

I ran into a problem when trying to bundle a package that depends on chalk using Rollup. I don't know if this is considered a bug but I think a small change could make chalk more portable by supporting the default config.

To reproduce

yarn add rollup @rollup/plugin-node-resolve chalk

// rollup.config.mjs
import nodeResolver from "@rollup/plugin-node-resolve";

export default {
  input: "./chalk-test.mjs",
  output: [
    {
      file: "dist/chalk-test.mjs",
      format: "es",
    },
  ],
  plugins: [
    nodeResolver({
      //   exportConditions: ["node"],
    }),
  ],
};
// chalk-test.mjs
import chalk from "chalk";

console.log(chalk.blue("Hello world!"));

Now after bundling and running the resulting bundle we get an error:

$ yarn rollup -c rollup.config.mjs
$ yarn node dist/chalk-test.mjs
/dist/chalk-test.mjs:225
const isBlinkBasedBrowser = navigator.userAgentData
                            ^

ReferenceError: navigator is not defined
    at file:///media/index/Dev/js/worms/packages/chalk-test/dist/chalk-test.mjs:225:29
    at ModuleJob.run (node:internal/modules/esm/module_job:194:25)

Node.js v19.1.0

The cause of this problem is the conditional import in package.json. Rollup by default selects the default version, which will includes the code that results in an error in a non-browser environment.

"imports": {
		"#ansi-styles": "./source/vendor/ansi-styles/index.js",
		"#supports-color": {
			"node": "./source/vendor/supports-color/index.js",
			"default": "./source/vendor/supports-color/browser.js"
		}
	},

This can be solved by manually adding node to exportConditions in the nodeResolver config (see https://www.npmjs.com/package/@rollup/plugin-node-resolve)

  plugins: [
    nodeResolver({
      exportConditions: ["node"],
    }),
  ],

This is not ideal because it is confusing and requires Rollup users to alter their config for all dependencies. I think ideally, the default version should not depend on the presence of the global navigator variable.

sindresorhus commented

This is a problem with Rollup or your Rollup config. You should take this up on the Rollup issue tracker, not here. Node.js recommends using node for Node.js and default as fallback for browsers, and this is what most isomorphic packages use.

jeroenmuller commented

Hi, thank you for your reply! As I said, I already found a workaround for my build (setting exportConditions: ["node"] to get my bundle to work in Node.js), the reason I decided to create this issue is that it might save others from encountering the same headache.

I do not think this is the fault of Rollup, which tries to make an environment-agnostic bundle for my library unless I tell it I want a node-specific or browser-specific version by configuring the export conditions accordingly (though their docs could be clearer). In his case, I was actally trying to create a portable bundle that can be used in both Node and browser environments. I understand that it is impossible to get full functionality of your package in this scenario because the Node code in https://github.com/chalk/supports-color/blob/main/index.js depends on node-specific APIs and should only be included when the node export condition is set. However, it would still be nice if the default export could degrade gracefully when it ends up in a non-browser environment instead of crashing, for example by defaulting to basic color support. I checked the Node.js docs again and also came across these related issues: chalk/supports-color#113 , #557 SBoudrias/Inquirer.js#1153

As per the Node.js documentation, if I understand it correctly, the default condition should offer an universal implementation that does not make assumptions about the environment.

When using environment branches, always include a "default" condition where possible. Providing a "default" condition ensures that any unknown JS environments are able to use this universal implementation, which helps avoid these JS environments from having to pretend to be existing environments in order to support packages with conditional exports. For this reason, using "node" and "default" condition branches is usually preferable to using "node" and "browser" condition branches.

In your package, this is not the case (the default version assumes that the browser-only navigator field is defined, which is not the case for all javascript environments). This means I have to pretend to be a node environment (which is my current solution) and give up the goal of building one bundle that can work across environments. In the end, creating a single bundle for multiple environments might be a rather niche goal, so I can also understand if you think it is not worth the trouble to support this.

  • 👍3