[parser] Fix symbol collision with require

Fixes https://github.com/oven-sh/bun/issues/674 Fixes https://github.com/oven-sh/bun/issues/382
2026-02-02 15:08:46 +00:00 · 2022-07-15 23:08:25 -07:00
parent e7246837a1
commit f7e9b91701
7 changed files with 63 additions and 40 deletions
--- a/17
+++ b/17
@@ -888,7 +888,18 @@ test-dev-bun-dev:
 	BUN_BIN=$(DEBUG_BUN) bash test/apps/bun-dev.sh
 	BUN_BIN=$(DEBUG_BUN) bash test/apps/bun-dev-index-html.sh

-test-all: test-install test-with-hmr test-no-hmr test-create-next test-create-react test-bun-run test-bun-install test-bun-dev
+test-bun-snapshot:
+	rm -rf test/bun.js/snapshots.js
+	touch test/bun.js/snapshots.js
+	$(foreach i,$(wildcard test/bun.js/*.snapshot.*),echo "" >> test/bun.js/snapshots.js; echo "// $i" >> test/bun.js/snapshots.js; $(RELEASE_BUN) build $i --platform=bun >> test/bun.js/snapshots.js;)
+
+test-dev-bun-snapshot:
+	rm -rf test/bun.js/snapshots.debug.js
+	touch test/bun.js/snapshots.debug.js
+	$(foreach i,$(wildcard test/bun.js/*.snapshot.*),echo "" >> test/bun.js/snapshots.debug.js; echo "// $i" >> test/bun.js/snapshots.debug.js; $(DEBUG_BUN) build $i --platform=bun >> test/bun.js/snapshots.debug.js;)
+
+
+test-all: test-install test-bun-snapshot test-with-hmr test-no-hmr test-create-next test-create-react test-bun-run test-bun-install test-bun-dev

 copy-test-node-modules:
 	rm -rf test/snippets/package-json-exports/node_modules || echo "";
@@ -943,7 +954,7 @@ test-dev-no-hmr: copy-test-node-modules
 test-dev-bun-run:
 	cd test/apps && BUN_BIN=$(DEBUG_BUN) bash bun-run-check.sh

-test-dev-all: test-dev-with-hmr test-dev-no-hmr test-dev-create-next test-dev-create-react test-dev-bun-run test-dev-bun-install test-dev-bun-dev
+test-dev-all: test-install test-dev-bun-snapshot test-dev-with-hmr test-dev-no-hmr test-dev-create-next test-dev-create-react test-dev-bun-run test-dev-bun-install test-dev-bun-dev
 test-dev-bunjs:

 test-dev: test-dev-with-hmr
@@ -1212,7 +1223,7 @@ bun-link-lld-release-no-lto:
 ifeq ($(OS_NAME),darwin)
 bun-link-lld-release-dsym:
 	$(DSYMUTIL) -o $(BUN_RELEASE_BIN).dSYM $(BUN_RELEASE_BIN)
-	-$(STRIP) $(BUN_RELEASE_BIN)
+	-$(STRIP) $(BUN_RELEASE_BIN) --wildcard -K _napi\*
 	cp $(BUN_RELEASE_BIN).o /tmp/bun-$(PACKAGE_JSON_VERSION).o

 copy-to-bun-release-dir-dsym:
--- a/src/js_parser.zig
+++ b/src/js_parser.zig
@@ -5015,6 +5015,11 @@ fn NewParser_(
        pub fn resolveCommonJSSymbols(p: *P) void {
            if (p.runtime_imports.__require) |*require| {
                p.resolveGeneratedSymbol(require);
+            } else if (p.symbols.items[p.require_ref.innerIndex()].use_count_estimate == 0 and
+                p.symbols.items[p.require_ref.innerIndex()].link.isNull())
+            {
+                // ensure our unused require() never collides with require()
+                p.symbols.items[p.require_ref.innerIndex()].original_name = "__require";
            }
        }

@@ -17736,6 +17741,7 @@ fn NewParser_(
                            .primary = p.require_ref,
                            .ref = declareSymbolMaybeGenerated(p, .other, logger.Loc.Empty, StaticSymbolName.List.__require.internal, true) catch unreachable,
                        };
+                        p.runtime_imports.put(name, p.runtime_imports.__require.?);
                        break :brk p.runtime_imports.__require.?.ref;
                    }
                    const generated_symbol = p.declareGeneratedSymbol(.other, name) catch unreachable;
--- a/src/linker.zig
+++ b/src/linker.zig
@@ -210,7 +210,6 @@ pub const Linker = struct {
        comptime allow_import_from_bundle: bool,
        comptime is_bun: bool,
    ) !void {
-        const supports_dynamic_require = comptime is_bun;
        const source_dir = file_path.sourceDir();
        var externals = std.ArrayList(u32).init(linker.allocator);
        var needs_bundle = false;
@@ -497,11 +496,8 @@ pub const Linker = struct {
                        if (loader != .napi and resolved_import.shouldAssumeCommonJS(import_record.kind)) {
                            import_record.do_commonjs_transform_in_printer = true;
                            import_record.module_id = @truncate(u32, std.hash.Wyhash.hash(0, path.pretty));
-
-                            if (comptime !supports_dynamic_require) {
-                                result.ast.needs_runtime = true;
-                                needs_require = true;
-                            }
+                            result.ast.needs_runtime = true;
+                            needs_require = true;
                        }
                    } else |err| {
                        switch (err) {
@@ -594,36 +590,34 @@ pub const Linker = struct {
            import_records = new_import_records;
        }

-        if (comptime !supports_dynamic_require) {
-            // We _assume_ you're importing ESM.
-            // But, that assumption can be wrong without parsing code of the imports.
-            // That's where in here, we inject
-            // > import {require} from 'bun:wrap';
-            // Since they definitely aren't using require, we don't have to worry about the symbol being renamed.
-            if (needs_require and !result.ast.uses_require_ref) {
-                result.ast.uses_require_ref = true;
-                require_part_import_clauses[0] = js_ast.ClauseItem{
-                    .alias = require_alias,
-                    .original_name = "",
-                    .alias_loc = logger.Loc.Empty,
-                    .name = js_ast.LocRef{
-                        .loc = logger.Loc.Empty,
-                        .ref = result.ast.require_ref,
-                    },
-                };
-
-                require_part_import_statement = js_ast.S.Import{
-                    .namespace_ref = Ref.None,
-                    .items = std.mem.span(&require_part_import_clauses),
-                    .import_record_index = result.ast.runtime_import_record_id.?,
-                };
-                require_part_stmts[0] = js_ast.Stmt{
-                    .data = .{ .s_import = &require_part_import_statement },
+        // We _assume_ you're importing ESM.
+        // But, that assumption can be wrong without parsing code of the imports.
+        // That's where in here, we inject
+        // > import {require} from 'bun:wrap';
+        // Since they definitely aren't using require, we don't have to worry about the symbol being renamed.
+        if (needs_require and !result.ast.uses_require_ref) {
+            result.ast.uses_require_ref = true;
+            require_part_import_clauses[0] = js_ast.ClauseItem{
+                .alias = require_alias,
+                .original_name = "",
+                .alias_loc = logger.Loc.Empty,
+                .name = js_ast.LocRef{
                    .loc = logger.Loc.Empty,
-                };
+                    .ref = result.ast.require_ref,
+                },
+            };

-                result.ast.prepend_part = js_ast.Part{ .stmts = std.mem.span(&require_part_stmts) };
-            }
+            require_part_import_statement = js_ast.S.Import{
+                .namespace_ref = Ref.None,
+                .items = std.mem.span(&require_part_import_clauses),
+                .import_record_index = result.ast.runtime_import_record_id.?,
+            };
+            require_part_stmts[0] = js_ast.Stmt{
+                .data = .{ .s_import = &require_part_import_statement },
+                .loc = logger.Loc.Empty,
+            };
+
+            result.ast.prepend_part = js_ast.Part{ .stmts = std.mem.span(&require_part_stmts) };
        }
    }

--- a/src/runtime.footer.bun.js
+++ b/src/runtime.footer.bun.js
@@ -3,7 +3,6 @@ export var __markAsModule = BUN_RUNTIME.__markAsModule;
 export var $$lzy = BUN_RUNTIME.$$lzy;
 export var __toModule = BUN_RUNTIME.__toModule;
 export var __commonJS = BUN_RUNTIME.__commonJS;
-export var __require = BUN_RUNTIME.__require;
 export var __name = BUN_RUNTIME.__name;
 export var __export = BUN_RUNTIME.__export;
 export var __reExport = BUN_RUNTIME.__reExport;
@@ -16,12 +15,12 @@ export var $$bun_runtime_json_parse = JSON.parse;
 export var __internalIsCommonJSNamespace =
  BUN_RUNTIME.__internalIsCommonJSNamespace;

-globalThis.require ||= function (moduleId) {
+export var __require = (globalThis.require ||= function (moduleId) {
  if (typeof moduleId === "string") {
    return import.meta.require(moduleId);
  }

  return BUN_RUNTIME.__require(moduleId);
-};
+});
 globalThis.__internalIsCommonJSNamespace ||=
  BUN_RUNTIME.__internalIsCommonJSNamespace;
--- a/test/bun.js/module-require.snapshot.js
+++ b/test/bun.js/module-require.snapshot.js
@@ -0,0 +1 @@
+console.log(module.require("react"));
--- a/test/bun.js/require-referenceerror.snapshot.js
+++ b/test/bun.js/require-referenceerror.snapshot.js
@@ -0,0 +1,5 @@
+/**
+ * https://github.com/oven-sh/bun/issues/685
+ */
+import { v4 as uuidv4 } from "uuid";
+Bun.inspect(uuidv4());
--- a/test/bun.js/tiny-typed-emitter.snapshot.js
+++ b/test/bun.js/tiny-typed-emitter.snapshot.js
@@ -0,0 +1,7 @@
+/**
+ * https://github.com/oven-sh/bun/issues/453
+ */
+import { createRequire as topLevelCreateRequire } from "module";
+import { TypedEmitter as TypedEmitter7 } from "tiny-typed-emitter";
+
+const require = topLevelCreateRequire(import.meta.url);