Babel Environment set to true breaks library configurations about knip HOT 8 CLOSED

By the way I try to minimize reading things like process.env to maximize being runtime agnostic (e.g. run in Bun, Deno, whatnot).

from knip.

By "breaks" you mean currentEnv is true instead of e.g. development, right?

Other than that, the idea seems sound to me: Knip will receive a configuration where all dependencies for all environments are included. How else could Knip figure this out in a single run? In general you don't want to run Knip multiple times I guess.

Having that said, and looking at Babel source code, I think the function should be something like this:

{
  env: (env: string) => typeof env === 'string' ? true : 'development'
}

Would you agree?

from knip.

@webpro yes, completely. I'm not entirely sure if the default should be 'development'.

If we can use the environment variable that is actually passed to knip, the better. If not, 'development' should work.

from knip.

That default value I got here: https://github.com/babel/babel/blob/main/packages/babel-core/src/config/helpers/environment.ts

The fix will come with the next release.

from knip.

@webpro Got it. Thank you 🙌

from knip.

@webpro Now that I think about it, I think returning true for all environments might cause some babel configurations to be invalid. Developers use isDevelopment, isProduction kind of variables to configure plugins for babel. If we are returning just true for all environments, I would assume this causes at least some configurations to be invalid.

I would expect something like Babel's own implementation of the function.

// Since we decided we aren't fetching from process
const defaultEnv = "development";

const env: EnvFunction = (
  value: string | string[] | (<T>(babelEnv: string) => T)
) => {
  if (typeof value === "undefined") return defaultEnv;
  if (typeof value === "function") {
    return value(defaultEnv);
  }
  return (Array.isArray(value) ? value : [value]).some((entry) => {
    if (typeof entry !== "string") {
      throw new Error("Unexpected non-string value");
    }
    return entry === defaultEnv;
  });
};

from knip.

Other than that, the idea seems sound to me: Knip will receive a configuration where all dependencies for all environments are included. How else could Knip figure this out in a single run? In general you don't want to run Knip multiple times I guess.

Yes in theory things might break, but so far so good, haven't received other issues around this.

from knip.

🚀 This issue has been resolved in v5.10.0-canary.1. See Release 5.10.0-canary.1 for release notes.

Using Knip in a commercial project? Please consider sponsoring me.

from knip.