bun icon indicating copy to clipboard operation
bun copied to clipboard

`.env` loader injects `true`,`false` as boolean instead of strings

Open rsc1975 opened this issue 2 years ago • 4 comments

Version

0.1.11 (canary)

Platform

Linux ThinkPad-Rob 5.15.57.1-microsoft-standard-WSL2 #1 SMP Wed Jul 27 02:20:31 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

What steps will reproduce the bug?

File sandbox.ts with content:

const quiet = process.env.QUIET === "true";
console.log('quiet:', quiet);
console.log('process.env.QUIET:', process.env.QUIET, typeof process.env.QUIET);

Run the file with:

$ QUIET=true bun run sandbox.ts

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior?

The object process.env is a Record<string, string> all variables values should be stored as strings.

The output should be:

quiet: true
process.env.QUIET: true string

What do you see instead?

This output:

quiet: false
process.env.QUIET: true boolean

Additional information

It only happens with boolean values, with numbers works as expected.

rsc1975 avatar Aug 31 '22 10:08 rsc1975

Bun is part bundler/transpiler and part runtime

The behavior you're seeing is an optimization for dead code elimination. It's not exactly a bug (but is clearly confusing sometimes). process.env.QUIET simplifies to "true" and then "true" === "true" simplifies to true. Note how process.env.QUIET is a string if evaluated at runtime

image

If you wanted to coerce it to always be a string, you could do "" + process.env.QUIET

Jarred-Sumner avatar Sep 01 '22 07:09 Jarred-Sumner

But @Jarred-Sumner , that code works different in node (and deno), where quite var is resolved as true, but in Bun is false, that sounds not correct. In other words, the problem is that in Bun quite local variable is resolved as false, and that is not correct according to specs.

rsc1975 avatar Sep 01 '22 07:09 rsc1975

Ah, yes you are correct. This is a bug in the .env parser. It is coercing boolean strings to boolean values when it should keep them as strings

image

Jarred-Sumner avatar Sep 01 '22 07:09 Jarred-Sumner

It looks like the bug is in one of these places

https://github.com/oven-sh/bun/blob/f023b89b732db0aff24445acbbe39c366d13118d/src/env_loader.zig#L540-L562

https://github.com/oven-sh/bun/blob/f023b89b732db0aff24445acbbe39c366d13118d/src/env_loader.zig#L588-L610

https://github.com/oven-sh/bun/blob/f023b89b732db0aff24445acbbe39c366d13118d/src/env_loader.zig#L638-L660

https://github.com/oven-sh/bun/blob/f023b89b732db0aff24445acbbe39c366d13118d/src/env_loader.zig#L692-L761

It also causes 0 and 1 to be numbers instead of strings and "undefined" to be undefined.

I wonder what this is for? It was clearly written out four times, so it probably has some use. I don't see any test cases covering it

A visual reproduction:

const q = "QUIET";
console.log({a: process.env.QUIET, b: process.env[q]});

pfgithub avatar Sep 01 '22 15:09 pfgithub