diff --git a/src/bun.js/bindings/BunString.cpp b/src/bun.js/bindings/BunString.cpp index ed05c09583..63cd1b94a1 100644 --- a/src/bun.js/bindings/BunString.cpp +++ b/src/bun.js/bindings/BunString.cpp @@ -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); } diff --git a/src/cli.zig b/src/cli.zig index c34a4344cc..7904a1e93d 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -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); diff --git a/src/cli/test_command.zig b/src/cli/test_command.zig index 6ff87ba3be..9518d36256 100644 --- a/src/cli/test_command.zig +++ b/src/cli/test_command.zig @@ -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(); diff --git a/src/env_loader.zig b/src/env_loader.zig index 835fe2487a..bc2bedbe92 100644 --- a/src/env_loader.zig +++ b/src/env_loader.zig @@ -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 { diff --git a/src/options.zig b/src/options.zig index 6cffead70a..eb79fe7385 100644 --- a/src/options.zig +++ b/src/options.zig @@ -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; } diff --git a/test/cli/hot/hot.test.ts b/test/cli/hot/hot.test.ts index 50a38ac68a..36ecd319e3 100644 --- a/test/cli/hot/hot.test.ts +++ b/test/cli/hot/hot.test.ts @@ -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"; diff --git a/test/cli/run/env.test.ts b/test/cli/run/env.test.ts index a29b85679f..3bd3c467d9 100644 --- a/test/cli/run/env.test.ts +++ b/test/cli/run/env.test.ts @@ -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", + ); + }); +}); diff --git a/test/cli/run/transpiler-cache.test.ts b/test/cli/run/transpiler-cache.test.ts index 5845c9a162..1885d71a03 100644 --- a/test/cli/run/transpiler-cache.test.ts +++ b/test/cli/run/transpiler-cache.test.ts @@ -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); + }); });