fix: do not inline process.env during bun run and bun test (#7614)

* env stuff

* ok

* undo

* remove unused param

* resolve review comment

* dupe

* it compiles now i promise
This commit is contained in:
dave caruso
2023-12-13 06:05:03 -08:00
committed by GitHub
parent 38c6575dc8
commit 5029fc8564
8 changed files with 193 additions and 89 deletions

View File

@@ -509,9 +509,7 @@ WTF::String BunString::toWTFString(ZeroCopyTag) const
}
if (this->tag == BunStringTag::WTFStringImpl) {
#if BUN_DEBUG
RELEASE_ASSERT(this->impl.wtf->refCount() > 0);
#endif
ASSERT(this->impl.wtf->refCount() > 0);
return WTF::String(this->impl.wtf);
}

View File

@@ -79,7 +79,7 @@ pub const LoaderColonList = ColonListType(Api.Loader, Arguments.loader_resolver)
pub const DefineColonList = ColonListType(string, Arguments.noop_resolver);
fn invalidTarget(diag: *clap.Diagnostic, _target: []const u8) noreturn {
@setCold(true);
diag.name.long = "--target";
diag.name.long = "target";
diag.arg = _target;
diag.report(Output.errorWriter(), error.InvalidTarget) catch {};
std.process.exit(1);

View File

@@ -637,6 +637,16 @@ pub const TestCommand = struct {
vm.argv = ctx.passthrough;
vm.preload = ctx.preloads;
vm.bundler.options.rewrite_jest_for_tests = true;
vm.bundler.options.env.behavior = .load_all_without_inlining;
const node_env_entry = try env_loader.map.getOrPutWithoutValue("NODE_ENV");
if (!node_env_entry.found_existing) {
node_env_entry.key_ptr.* = try env_loader.allocator.dupe(u8, node_env_entry.key_ptr.*);
node_env_entry.value_ptr.* = .{
.value = try env_loader.allocator.dupe(u8, "test"),
.conditional = false,
};
}
try vm.bundler.configureDefines();

View File

@@ -224,7 +224,7 @@ pub const Loader = struct {
behavior: Api.DotEnvBehavior,
prefix: string,
allocator: std.mem.Allocator,
) ![]u8 {
) !void {
var iter = this.map.iter();
var key_count: usize = 0;
var string_map_hashes = try allocator.alloc(u64, framework_defaults.keys.len);
@@ -367,8 +367,6 @@ pub const Loader = struct {
_ = try to_json.getOrPutValue(key, value);
}
}
return key_buf;
}
pub fn init(map: *Map, allocator: std.mem.Allocator) Loader {

View File

@@ -1107,14 +1107,6 @@ pub const Timings = struct {
};
pub const DefaultUserDefines = struct {
pub const HotModuleReloading = struct {
pub const Key = "process.env.BUN_HMR_ENABLED";
pub const Value = "true";
};
pub const HotModuleReloadingVerbose = struct {
pub const Key = "process.env.BUN_HMR_VERBOSE";
pub const Value = "true";
};
// This must be globally scoped so it doesn't disappear
pub const NodeEnv = struct {
pub const Key = "process.env.NODE_ENV";
@@ -1129,16 +1121,13 @@ pub const DefaultUserDefines = struct {
pub fn definesFromTransformOptions(
allocator: std.mem.Allocator,
log: *logger.Log,
_input_define: ?Api.StringMap,
hmr: bool,
maybe_input_define: ?Api.StringMap,
target: Target,
loader: ?*DotEnv.Loader,
env_loader: ?*DotEnv.Loader,
framework_env: ?*const Env,
NODE_ENV: ?string,
debugger: bool,
) !*defines.Define {
_ = debugger;
var input_user_define = _input_define orelse std.mem.zeroes(Api.StringMap);
var input_user_define = maybe_input_define orelse std.mem.zeroes(Api.StringMap);
var user_defines = try stringHashMapFromArrays(
defines.RawDefines,
@@ -1150,73 +1139,71 @@ pub fn definesFromTransformOptions(
var environment_defines = defines.UserDefinesArray.init(allocator);
defer environment_defines.deinit();
if (loader) |_loader| {
if (framework_env) |framework| {
_ = try _loader.copyForDefine(
defines.RawDefines,
&user_defines,
defines.UserDefinesArray,
&environment_defines,
framework.toAPI().defaults,
framework.behavior,
framework.prefix,
allocator,
);
} else {
_ = try _loader.copyForDefine(
defines.RawDefines,
&user_defines,
defines.UserDefinesArray,
&environment_defines,
std.mem.zeroes(Api.StringMap),
Api.DotEnvBehavior.disable,
"",
allocator,
);
var behavior: Api.DotEnvBehavior = .disable;
load_env: {
const env = env_loader orelse break :load_env;
const framework = framework_env orelse break :load_env;
if (Environment.allow_assert) {
std.debug.assert(framework.behavior != ._none);
}
behavior = framework.behavior;
if (behavior == .load_all_without_inlining or behavior == .disable)
break :load_env;
try env.copyForDefine(
defines.RawDefines,
&user_defines,
defines.UserDefinesArray,
&environment_defines,
framework.toAPI().defaults,
framework.behavior,
framework.prefix,
allocator,
);
}
var quoted_node_env: string = brk: {
if (NODE_ENV) |node_env| {
if (node_env.len > 0) {
if ((strings.startsWithChar(node_env, '"') and strings.endsWithChar(node_env, '"')) or
(strings.startsWithChar(node_env, '\'') and strings.endsWithChar(node_env, '\'')))
{
break :brk node_env;
}
if (behavior != .load_all_without_inlining) {
var quoted_node_env: string = brk: {
if (NODE_ENV) |node_env| {
if (node_env.len > 0) {
if ((strings.startsWithChar(node_env, '"') and strings.endsWithChar(node_env, '"')) or
(strings.startsWithChar(node_env, '\'') and strings.endsWithChar(node_env, '\'')))
{
break :brk node_env;
}
// avoid allocating if we can
if (strings.eqlComptime(node_env, "production")) {
break :brk "\"production\"";
} else if (strings.eqlComptime(node_env, "development")) {
break :brk "\"development\"";
} else if (strings.eqlComptime(node_env, "test")) {
break :brk "\"test\"";
} else {
break :brk try std.fmt.allocPrint(allocator, "\"{s}\"", .{node_env});
// avoid allocating if we can
if (strings.eqlComptime(node_env, "production")) {
break :brk "\"production\"";
} else if (strings.eqlComptime(node_env, "development")) {
break :brk "\"development\"";
} else if (strings.eqlComptime(node_env, "test")) {
break :brk "\"test\"";
} else {
break :brk try std.fmt.allocPrint(allocator, "\"{s}\"", .{node_env});
}
}
}
break :brk "\"development\"";
};
_ = try user_defines.getOrPutValue(
"process.env.NODE_ENV",
quoted_node_env,
);
_ = try user_defines.getOrPutValue(
"process.env.BUN_ENV",
quoted_node_env,
);
// Automatically set `process.browser` to `true` for browsers and false for node+js
// This enables some extra dead code elimination
if (target.processBrowserDefineValue()) |value| {
_ = try user_defines.getOrPutValue(DefaultUserDefines.ProcessBrowserDefine.Key, value);
}
break :brk "\"development\"";
};
_ = try user_defines.getOrPutValue(
"process.env.NODE_ENV",
quoted_node_env,
);
_ = try user_defines.getOrPutValue(
"process.env.BUN_ENV",
quoted_node_env,
);
if (hmr) {
try user_defines.put(DefaultUserDefines.HotModuleReloading.Key, DefaultUserDefines.HotModuleReloading.Value);
}
// Automatically set `process.browser` to `true` for browsers and false for node+js
// This enables some extra dead code elimination
if (target.processBrowserDefineValue()) |value| {
_ = try user_defines.getOrPutValue(DefaultUserDefines.ProcessBrowserDefine.Key, value);
}
if (target.isBun()) {
@@ -1525,7 +1512,6 @@ pub const BundleOptions = struct {
allocator,
this.log,
this.transform_options.define,
this.transform_options.serve orelse false,
this.target,
loader_,
env,
@@ -1543,7 +1529,6 @@ pub const BundleOptions = struct {
break :node_env "\"development\"";
},
this.debugger,
);
this.defines_loaded = true;
}

View File

@@ -1,6 +1,6 @@
import { spawn } from "bun";
import { expect, it } from "bun:test";
import { bunExe, bunEnv } from "harness";
import { bunExe, bunEnv, tempDirWithFiles, bunRun, bunRunAsScript } from "harness";
import { readFileSync, renameSync, rmSync, unlinkSync, writeFileSync } from "fs";
import { join } from "path";

View File

@@ -127,7 +127,7 @@ describe(".env file is loaded", () => {
const { stdout } = bunTest(`${dir}/index.test.ts`);
expect(stdout).toBe("false");
});
test.todo("NODE_ENV is automatically set to test within bun test", () => {
test("NODE_ENV is automatically set to test within bun test", () => {
const dir = tempDirWithFiles("dotenv", {
"index.test.ts": "console.log(process.env.NODE_ENV);",
});
@@ -365,7 +365,7 @@ test(".env special characters 1 (issue #2823)", () => {
expect(stdout).toBe("[a] [c$v]");
});
test.todo("env escaped quote (issue #2484)", () => {
test("env escaped quote (issue #2484)", () => {
const dir = tempDirWithFiles("env-issue-2484", {
"index.ts": "console.log(process.env.VALUE, process.env.VALUE2);",
});
@@ -424,7 +424,8 @@ test("#3911", () => {
});
describe("boundary tests", () => {
test("src boundary", () => {
// TODO: this is a regression in bun ~1.0.15 ish
test.todo("src boundary", () => {
const dir = tempDirWithFiles("dotenv", {
".env": 'KEY="a\\n"',
"index.ts": "console.log(process.env.KEY);",
@@ -579,3 +580,102 @@ describe("--env-file", () => {
expect(res.stdout).toBe("");
});
});
describe("process.env is not inlined", () => {
test("basic case", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.ts": `process.env.NODE_ENV = "production";
process.env.YOLO = "woo!";
console.log(process.env.NODE_ENV, process.env.YOLO);`,
});
expect(
bunRun(path.join(tmp, "index.ts"), {
NODE_ENV: undefined,
YOLO: "boo",
}).stdout,
).toBe("production woo!");
});
test("pass explicit NODE_ENV case", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.ts": `console.log(process.env.NODE_ENV);
process.env.NODE_ENV = "development";
process.env.YOLO = "woo!";
console.log(process.env.NODE_ENV, process.env.YOLO);`,
});
expect(
bunRun(path.join(tmp, "index.ts"), {
NODE_ENV: "production",
YOLO: "boo",
}).stdout,
).toBe("production\ndevelopment woo!");
});
test("pass weird NODE_ENV case", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.ts": `console.log(process.env.NODE_ENV);
process.env.NODE_ENV = "development";
process.env.YOLO = "woo!";
console.log(process.env.NODE_ENV, process.env.YOLO);`,
});
expect(
bunRun(path.join(tmp, "index.ts"), {
NODE_ENV: "buh",
YOLO: "boo",
}).stdout,
).toBe("buh\ndevelopment woo!");
});
test("in bun test", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.test.ts": `test("my test", () => {
console.log(process.env.NODE_ENV);
process.env.NODE_ENV = "development";
process.env.YOLO = "woo!";
console.log(process.env.NODE_ENV, process.env.YOLO);
});`,
});
expect(
bunTest(path.join(tmp, "index.test.ts"), {
YOLO: "boo",
}).stdout,
).toBe("test\ndevelopment woo!");
});
test("in bun test with explicit setting", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.test.ts": `test("my test", () => {
console.log(process.env.NODE_ENV);
process.env.NODE_ENV = "development";
process.env.YOLO = "woo!";
console.log(process.env.NODE_ENV, process.env.YOLO);
});`,
});
expect(
bunTest(path.join(tmp, "index.test.ts"), {
YOLO: "boo",
NODE_ENV: "production",
}).stdout,
).toBe("production\ndevelopment woo!");
});
test("in bun test with dynamic access", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.test.ts": `const dynamic = () => require('process')['e' + String('nv')];
test("my test", () => {
console.log(dynamic().NODE_ENV);
process.env.NODE_ENV = "production";
console.log(dynamic().NODE_ENV);
});`,
});
expect(bunTest(path.join(tmp, "index.test.ts"), {}).stdout).toBe("test\nproduction");
});
test("in bun test with dynamic access + explicit set", () => {
const tmp = tempDirWithFiles("env-inlining", {
"index.test.ts": `const dynamic = () => require('process')['e' + String('nv')];
test("my test", () => {
console.log(dynamic().NODE_ENV);
process.env.NODE_ENV = "production";
console.log(dynamic().NODE_ENV);
});`,
});
expect(bunTest(path.join(tmp, "index.test.ts"), { NODE_ENV: "development" }).stdout).toBe(
"development\nproduction",
);
});
});

View File

@@ -6,10 +6,10 @@ import { bunEnv, bunExe, bunRun } from "harness";
import { tmpdir } from "os";
import { join } from "path";
function dummyFile(size: number, cache_bust: string, value: string) {
function dummyFile(size: number, cache_bust: string, value: string | { code: string }) {
const data = Buffer.alloc(size);
data.write("/*" + cache_bust);
const end = `*/\nconsole.log(${JSON.stringify(value)});`;
const end = `*/\nconsole.log(${(value as any).code ?? JSON.stringify(value)});`;
data.fill("*", 2 + cache_bust.length, size - end.length, "utf-8");
data.write(end, size - end.length, "utf-8");
return data;
@@ -160,4 +160,17 @@ describe("transpiler cache", () => {
chmodSync(join(cache_dir), "777");
});
test("does not inline process.env", () => {
writeFileSync(
join(temp_dir, "a.js"),
dummyFile((50 * 1024 * 1.5) | 0, "1", { code: "process.env.NODE_ENV, process.env.HELLO" }),
);
const a = bunRun(join(temp_dir, "a.js"), { ...env, NODE_ENV: undefined, HELLO: "1" });
expect(a.stdout == "development 1");
assert(existsSync(cache_dir));
expect(newCacheCount()).toBe(1);
const b = bunRun(join(temp_dir, "a.js"), { ...env, NODE_ENV: "production", HELLO: "5" });
expect(b.stdout == "production 5");
expect(newCacheCount()).toBe(0);
});
});