Supress Webpack warning for conditional require

[carlonoelle]

As stated in #67 some bundlers throw the following error/warning:

⚠ ./node_modules/tailwindcss/node_modules/picocolors/picocolors.js Critical dependency: require function is used in a way in which dependencies cannot be statically extracted

As you can see for me the error came up throuth the tailwindcss module, which i wanted to use inside a NextJS React project with nextjs@rc and react@rc which will become react 19 and nextjs 15 respectively.

NextJS uses Webpack, and Webpack for some reason has a setting called "unknownContextCritical" which checks after conditional require statements, which will then throw errors: https://github.com/webpack/docs/wiki/configuration (search the page after it)

To fix the error, simply move the require outside of the condition, its not the nicest since now the require is executed everytime, i know. But for me this is the only fix, i also created a patch-packages patch, see here (only works if you also get the error by using tailwind, otherwise you would need to patch the package yourself):

// patches/tailwindcss++picocolors+1.0.1.patch

diff --git a/node_modules/tailwindcss/node_modules/picocolors/picocolors.js b/node_modules/tailwindcss/node_modules/picocolors/picocolors.js
index 8b8a23e..0b7a529 100644
--- a/node_modules/tailwindcss/node_modules/picocolors/picocolors.js
+++ b/node_modules/tailwindcss/node_modules/picocolors/picocolors.js
@@ -1,11 +1,12 @@
 let argv = process.argv || [],
 	env = process.env
+let tty = require("tty")
 let isColorSupported =
 	!("NO_COLOR" in env || argv.includes("--no-color")) &&
 	("FORCE_COLOR" in env ||
 		argv.includes("--color") ||
 		process.platform === "win32" ||
-		(require != null && require("tty").isatty(1) && env.TERM !== "dumb") ||
+		(tty && tty.isatty(1) && env.TERM !== "dumb") ||
 		"CI" in env)
 
 let formatter =

[alexeyraspopov]

As much as I want to just merge this, using require() unconditionally going to break Edge Runtime, see #54.

How did you end up with webpack trying to analyze (bundle?) picocolors?

[carlonoelle]

Didn't think of that. I do not have much experience handling multi runtime compatibility with a commonjs package (or any package).

My guess would be that the problem would be solved by moving the code to import/export as an ES module (at least vercel tells us that: https://vercel.com/guides/library-sdk-compatible-with-vercel-edge-runtime-and-functions#unsupported-apis), but since that crashes out the older node versions that do not support esm you would need to upgrade the min engine version.

Would have to look into how other packages handle this, maybe you can have two entry files mentioned in package.json, one for commonjs one for esm?

[alexeyraspopov]

It's getting closer and closer to the moment to consider switching to ESM package, so I'll be focusing on the strategy for 2.0, that hopefully should address these kind of problem. I wished the ecosystem would move faster. But a node package (ie users projects) are still CJS by default and shipping ESM-only package means the users must be aware of potential changes on their side. Shipping CJS+ESM package seems like a waste though and I wish it could be avoided.

[carlonoelle]

Just saw this: https://github.com/component/escape-html/pull/30/files#diff-47407fecafdf5f5cd55403c3de457833ddf9b6fab45253c04e1dc4c7cb4495b1

Seems like you can port to ESM with just a simple rollbar config, maybe worth a look

[webdiscus]

nice

[alexeyraspopov]

@webdiscus nice copypaste

How do you even blow up a simple script to this amount of code? Wow

[alexeyraspopov]

@carlonoelle I may have figured a way to suppress this warning without changing much things around. We can alias require to avoid it to be statically analyzed. If you can apply this patch to test this in your setup I'd really appreciate it:

let argv = process.argv || [],
-	env = process.env
+	env = process.env, req 
let isColorSupported =
	!("NO_COLOR" in env || argv.includes("--no-color")) &&
	("FORCE_COLOR" in env ||
		argv.includes("--color") ||
		process.platform === "win32" ||
-		(require != null && require("tty").isatty(1) && env.TERM !== "dumb") ||
+		(require != null && (req = require) && req("tty").isatty(1) && env.TERM !== "dumb") || 
		"CI" in env)

[carlonoelle]

Hey @alexeyraspopov ,

sadly thats not working, i regain the errror:

I will revert to my previous patch, since for me this is a dependency of tailwind which runs at build time not in an edge runtime

[alexeyraspopov]

Version 1.1.1 is published and no longer contains a require() call.