diff --git a/src/ast/P.zig b/src/ast/P.zig index 718dc2cf93..3debdebf9b 100644 --- a/src/ast/P.zig +++ b/src/ast/P.zig @@ -5434,7 +5434,7 @@ pub fn NewParser_( }; } - pub fn isDotDefineMatch(noalias p: *P, expr: Expr, parts: []const string) bool { + pub fn isDotDefineMatch(noalias p: *P, expr: Expr, parts: []const string, define_data: *const DefineData) bool { switch (expr.data) { .e_dot => |ex| { if (parts.len > 1) { @@ -5445,7 +5445,7 @@ pub fn NewParser_( // Intermediates must be dot expressions const last = parts.len - 1; const is_tail_match = strings.eql(parts[last], ex.name); - return is_tail_match and p.isDotDefineMatch(ex.target, parts[0..last]); + return is_tail_match and p.isDotDefineMatch(ex.target, parts[0..last], define_data); } }, .e_import_meta => { @@ -5463,7 +5463,7 @@ pub fn NewParser_( const last = parts.len - 1; const is_tail_match = strings.eql(parts[last], index.index.data.e_string.slice(p.allocator)); - return is_tail_match and p.isDotDefineMatch(index.target, parts[0..last]); + return is_tail_match and p.isDotDefineMatch(index.target, parts[0..last], define_data); } }, .e_identifier => |ex| { @@ -5484,7 +5484,9 @@ pub fn NewParser_( // when there's actually no symbol by that name, we return Ref.None // If a symbol had already existed by that name, we return .unbound - return (result.ref.isNull() or p.symbols.items[result.ref.innerIndex()].kind == .unbound); + // If `replace_even_if_shadowed` is set (e.g., for --env defines), we + // allow the replacement even when the symbol is shadowed by a local variable. + return (result.ref.isNull() or p.symbols.items[result.ref.innerIndex()].kind == .unbound or define_data.replace_even_if_shadowed()); } }, else => {}, diff --git a/src/ast/visitExpr.zig b/src/ast/visitExpr.zig index cc8bcb1b49..cdd7dc1c79 100644 --- a/src/ast/visitExpr.zig +++ b/src/ast/visitExpr.zig @@ -67,9 +67,9 @@ pub fn VisitExpr( const is_delete_target = p.delete_target == .e_import_meta; if (p.define.dots.get("meta")) |meta| { - for (meta) |define| { + for (meta) |*define| { // TODO: clean up how we do define matches - if (p.isDotDefineMatch(expr, define.parts)) { + if (p.isDotDefineMatch(expr, define.parts, &define.data)) { // Substitute user-specified defines return p.valueForDefine(expr.loc, in.assign_target, is_delete_target, &define.data); } @@ -832,7 +832,7 @@ pub fn VisitExpr( if (p.define.dots.get(e_.name)) |parts| { for (parts) |*define| { - if (p.isDotDefineMatch(expr, define.parts)) { + if (p.isDotDefineMatch(expr, define.parts, &define.data)) { if (in.assign_target == .none) { // Substitute user-specified defines if (!define.data.valueless()) { diff --git a/src/defines.zig b/src/defines.zig index 4886eb95a7..8d3ceeee2f 100644 --- a/src/defines.zig +++ b/src/defines.zig @@ -27,7 +27,7 @@ pub const DefineData = struct { flags: Flags = .{}, pub const Flags = packed struct(u8) { - _padding: u3 = 0, + _padding: u2 = 0, valueless: bool = false, @@ -36,6 +36,11 @@ pub const DefineData = struct { call_can_be_unwrapped_if_unused: js_ast.E.CallUnwrap = .never, method_call_must_be_replaced_with_undefined: bool = false, + + /// If true, this define will be replaced even if the base identifier + /// is shadowed by a local variable. This is used for `--env` defines + /// where the user explicitly opted into env replacement. + replace_even_if_shadowed: bool = false, }; pub const Options = struct { @@ -45,6 +50,7 @@ pub const DefineData = struct { can_be_removed_if_unused: bool = false, call_can_be_unwrapped_if_unused: js_ast.E.CallUnwrap = .never, method_call_must_be_replaced_with_undefined: bool = false, + replace_even_if_shadowed: bool = false, }; pub fn init(options: Options) DefineData { @@ -55,6 +61,7 @@ pub const DefineData = struct { .can_be_removed_if_unused = options.can_be_removed_if_unused, .call_can_be_unwrapped_if_unused = options.call_can_be_unwrapped_if_unused, .method_call_must_be_replaced_with_undefined = options.method_call_must_be_replaced_with_undefined, + .replace_even_if_shadowed = options.replace_even_if_shadowed, }, .original_name_ptr = if (options.original_name) |name| name.ptr else null, .original_name_len = if (options.original_name) |name| @truncate(name.len) else 0, @@ -90,6 +97,10 @@ pub const DefineData = struct { return self.flags.valueless; } + pub inline fn replace_even_if_shadowed(self: *const DefineData) bool { + return self.flags.replace_even_if_shadowed; + } + pub fn initBoolean(value: bool) DefineData { return .{ .value = .{ .e_boolean = .{ .value = value } }, diff --git a/src/env_loader.zig b/src/env_loader.zig index 03b08f5f15..d92545353c 100644 --- a/src/env_loader.zig +++ b/src/env_loader.zig @@ -416,6 +416,7 @@ pub const Loader = struct { .can_be_removed_if_unused = true, .call_can_be_unwrapped_if_unused = .if_unused, .value = expr_data, + .replace_even_if_shadowed = true, }), ); e_strings = e_strings[1..]; @@ -440,6 +441,7 @@ pub const Loader = struct { .can_be_removed_if_unused = true, .call_can_be_unwrapped_if_unused = .if_unused, .value = expr_data, + .replace_even_if_shadowed = true, }), ); e_strings = e_strings[1..]; @@ -471,6 +473,7 @@ pub const Loader = struct { .can_be_removed_if_unused = true, .call_can_be_unwrapped_if_unused = .if_unused, .value = expr_data, + .replace_even_if_shadowed = true, }), ); e_strings = e_strings[1..]; diff --git a/test/regression/issue/24348.test.ts b/test/regression/issue/24348.test.ts new file mode 100644 index 0000000000..d60e79097c --- /dev/null +++ b/test/regression/issue/24348.test.ts @@ -0,0 +1,69 @@ +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +// https://github.com/oven-sh/bun/issues/24348 +test("bundler replaces process.env even when 'process' is shadowed by a local variable", async () => { + using dir = tempDir("issue-24348", { + "main.js": ` +const works = () => console.log("Works: " + process.env.TEST_PUBLIC_ENV); + +const shouldAlsoWork = () => { + const process = { + env: { + OTHER_VAR: "123" + } + } + + console.log("Should also work: " + process.env.TEST_PUBLIC_ENV); +}; + +works(); +shouldAlsoWork(); +`, + }); + + // Bundle the file with --env pattern matching + await using bundleProc = Bun.spawn({ + cmd: [bunExe(), "build", "main.js", "--outfile", "out.js", "--env", "TEST_PUBLIC_*"], + env: { ...bunEnv, TEST_PUBLIC_ENV: "replaced_value" }, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + + const [bundleStdout, bundleStderr, bundleExitCode] = await Promise.all([ + bundleProc.stdout.text(), + bundleProc.stderr.text(), + bundleProc.exited, + ]); + + expect(bundleStderr).toBe(""); + expect(bundleExitCode).toBe(0); + + // Read the bundled output + const bundledCode = await Bun.file(`${dir}/out.js`).text(); + + // Both occurrences should be replaced with the env value + const matches = bundledCode.match(/"replaced_value"/g); + expect(matches).not.toBeNull(); + expect(matches?.length).toBe(2); + + // Verify the original process.env.TEST_PUBLIC_ENV is not in the output + expect(bundledCode).not.toContain("process.env.TEST_PUBLIC_ENV"); + + // Run the bundled code to verify it works + await using runProc = Bun.spawn({ + cmd: [bunExe(), "out.js"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + stdout: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([runProc.stdout.text(), runProc.stderr.text(), runProc.exited]); + + expect(stderr).toBe(""); + expect(stdout).toContain("Works: replaced_value"); + expect(stdout).toContain("Should also work: replaced_value"); + expect(exitCode).toBe(0); +});