Skip to content

Commit

Permalink
transpiler: dont inline import.meta.require (#16222)
Browse files Browse the repository at this point in the history
  • Loading branch information
@paperclover
paperclover authored Jan 8, 2025
1 parent d68d0cc commit a3cbf97
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 111 deletions.
3 changes: 2 additions & 1 deletion src/bun.js/RuntimeTranspilerCache.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
/// Version 10: Constant folding for ''.charCodeAt(n)
/// Version 11: Fix \uFFFF printing regression
/// Version 12: "use strict"; makes it CommonJS if we otherwise don't know which one to pick.
const expected_version = 12;
/// Version 13: Hoist `import.meta.require` definition, see #15738
const expected_version = 13;

const bun = @import("root").bun;
const std = @import("std");
Expand Down
4 changes: 0 additions & 4 deletions src/bun.js/api/JSTranspiler.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,6 @@ fn transformOptionsFromJSC(globalObject: JSC.C.JSContextRef, temp_allocator: std
}

transpiler.runtime.allow_runtime = false;
transpiler.runtime.use_import_meta_require = switch (transpiler.transform.target orelse .browser) {
.bun, .bun_macro => true,
else => false,
};

if (try object.getTruthy(globalThis, "macro")) |macros| {
macros: {
Expand Down
45 changes: 25 additions & 20 deletions src/bundler/bundle_v2.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -327,10 +327,15 @@ fn genericPathWithPrettyInitialized(path: Fs.Path, target: options.Target, top_l
// TODO: outbase
var buf: bun.PathBuffer = undefined;

const is_node = bun.strings.eqlComptime(path.namespace, "node");
if (is_node and bun.strings.hasPrefixComptime(path.text, NodeFallbackModules.import_path)) {
return path;
}

// "file" namespace should use the relative file path for its display name.
// the "node" namespace is also put through this code path so that the
// "node:" prefix is not emitted.
if (path.isFile() or bun.strings.eqlComptime(path.namespace, "node")) {
if (path.isFile() or is_node) {
const rel = bun.path.relativePlatform(top_level_dir, path.text, .loose, false);
var path_clone = path;
// stack-allocated temporary is not leaked because dupeAlloc on the path will
Expand Down Expand Up @@ -3464,10 +3469,12 @@ pub const ParseTask = struct {
// and then that is either replaced with the module itself, or an import to the
// runtime here.
const runtime_require = switch (target) {
// __require is intentionally not implemented here, as we
// always inline 'import.meta.require' and 'import.meta.require.resolve'
// Omitting it here acts as an extra assertion.
.bun, .bun_macro => "",
// Previously, Bun inlined `import.meta.require` at all usages. This broke
// code that called `fn.toString()` and parsed the code outside a module
// context.
.bun, .bun_macro =>
\\export var __require = import.meta.require;
,

.node =>
\\import { createRequire } from "node:module";
Expand Down Expand Up @@ -4406,10 +4413,9 @@ pub const ParseTask = struct {
opts.macro_context = &this.data.macro_context;
opts.package_version = task.package_version;

opts.features.auto_polyfill_require = output_format == .esm and !target.isBun();
opts.features.auto_polyfill_require = output_format == .esm;
opts.features.allow_runtime = !source.index.isRuntime();
opts.features.unwrap_commonjs_to_esm = output_format == .esm and FeatureFlags.unwrap_commonjs_to_esm;
opts.features.use_import_meta_require = target.isBun();
opts.features.top_level_await = output_format == .esm or output_format == .internal_bake_dev;
opts.features.auto_import_jsx = task.jsx.parse and transpiler.options.auto_import_jsx;
opts.features.trim_unused_imports = loader.isTypeScript() or (transpiler.options.trim_unused_imports orelse false);
Expand Down Expand Up @@ -7000,18 +7006,17 @@ pub const LinkerContext = struct {
try this.graph.generateSymbolImportAndUse(source_index, 0, module_ref, 1, Index.init(source_index));

// If this is a .napi addon and it's not node, we need to generate a require() call to the runtime
if (expr.data == .e_call and expr.data.e_call.target.data == .e_require_call_target and
if (expr.data == .e_call and
expr.data.e_call.target.data == .e_require_call_target and
// if it's commonjs, use require()
this.options.output_format != .cjs and
// if it's esm and bun, use import.meta.require(). the code for __require is not injected into the bundle.
!this.options.target.isBun())
this.options.output_format != .cjs)
{
this.graph.generateRuntimeSymbolImportAndUse(
try this.graph.generateRuntimeSymbolImportAndUse(
source_index,
Index.part(1),
"__require",
1,
) catch {};
);
}
},
else => {
Expand Down Expand Up @@ -7752,11 +7757,9 @@ pub const LinkerContext = struct {

continue;
} else {

// We should use "__require" instead of "require" if we're not
// generating a CommonJS output file, since it won't exist otherwise.
// Disabled for target bun because `import.meta.require` will be inlined.
if (shouldCallRuntimeRequire(output_format) and !this.resolver.opts.target.isBun()) {
if (shouldCallRuntimeRequire(output_format)) {
runtime_require_uses += 1;
}

Expand Down Expand Up @@ -9386,7 +9389,7 @@ pub const LinkerContext = struct {
var runtime_members = &runtime_scope.members;
const toCommonJSRef = c.graph.symbols.follow(runtime_members.get("__toCommonJS").?.ref);
const toESMRef = c.graph.symbols.follow(runtime_members.get("__toESM").?.ref);
const runtimeRequireRef = if (c.resolver.opts.target.isBun()) null else c.graph.symbols.follow(runtime_members.get("__require").?.ref);
const runtimeRequireRef = if (c.options.output_format == .cjs) null else c.graph.symbols.follow(runtime_members.get("__require").?.ref);

const result = c.generateCodeForFileInChunkJS(
&buffer_writer,
Expand Down Expand Up @@ -9855,10 +9858,11 @@ pub const LinkerContext = struct {
var runtime_members = &runtime_scope.members;
const toCommonJSRef = c.graph.symbols.follow(runtime_members.get("__toCommonJS").?.ref);
const toESMRef = c.graph.symbols.follow(runtime_members.get("__toESM").?.ref);
const runtimeRequireRef = if (c.resolver.opts.target.isBun() or c.options.output_format == .cjs) null else c.graph.symbols.follow(runtime_members.get("__require").?.ref);
const runtimeRequireRef = if (c.options.output_format == .cjs) null else c.graph.symbols.follow(runtime_members.get("__require").?.ref);

{
const print_options = js_printer.Options{
.bundling = true,
.indent = .{},
.has_run_symbol_renamer = true,

Expand Down Expand Up @@ -10015,7 +10019,7 @@ pub const LinkerContext = struct {

switch (c.options.output_format) {
.internal_bake_dev => {
const start = bun.bake.getHmrRuntime(if (c.options.target.isBun()) .server else .client);
const start = bun.bake.getHmrRuntime(if (c.options.target.isServerSide()) .server else .client);
j.pushStatic(start);
line_offset.advance(start);
},
Expand Down Expand Up @@ -12513,6 +12517,7 @@ pub const LinkerContext = struct {
};

const print_options = js_printer.Options{
.bundling = true,
// TODO: IIFE
.indent = .{},
.commonjs_named_exports = ast.commonjs_named_exports,
Expand Down Expand Up @@ -14099,7 +14104,7 @@ pub const LinkerContext = struct {
},
) catch unreachable;
}
} else if (c.resolver.opts.target == .browser and JSC.HardcodedModule.Aliases.has(next_source.path.pretty, .browser)) {
} else if (c.resolver.opts.target == .browser and bun.strings.hasPrefixComptime(next_source.path.text, NodeFallbackModules.import_path)) {
c.log.addRangeErrorFmtWithNote(
source,
r,
Expand Down
2 changes: 0 additions & 2 deletions src/js_ast.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6917,8 +6917,6 @@ pub const Ast = struct {
wrapper_ref: Ref = Ref.None,
require_ref: Ref = Ref.None,

prepend_part: ?Part = null,

// These are used when bundling. They are filled in during the parser pass
// since we already have to traverse the AST then anyway and the parser pass
// is conveniently fully parallelized.
Expand Down
31 changes: 16 additions & 15 deletions src/js_parser.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18863,22 +18863,20 @@ fn NewParser_(
}
},
.e_import_meta => {
// Make `import.meta.url` side effect free.
if (strings.eqlComptime(name, "url")) {
return p.newExpr(
E.Dot{
.target = target,
.name = name,
.name_loc = name_loc,
.can_be_removed_if_unused = true,
},
target.loc,
);
}

if (strings.eqlComptime(name, "main")) {
return p.valueForImportMetaMain(false, target.loc);
}

// Make all property accesses on `import.meta.url` side effect free.
return p.newExpr(
E.Dot{
.target = target,
.name = name,
.name_loc = name_loc,
.can_be_removed_if_unused = true,
},
target.loc,
);
},
.e_require_call_target => {
if (strings.eqlComptime(name, "main")) {
Expand Down Expand Up @@ -23749,8 +23747,11 @@ fn NewParser_(
.force_cjs_to_esm = p.unwrap_all_requires or exports_kind == .esm_with_dynamic_fallback_from_cjs,
.uses_module_ref = p.symbols.items[p.module_ref.inner_index].use_count_estimate > 0,
.uses_exports_ref = p.symbols.items[p.exports_ref.inner_index].use_count_estimate > 0,
.uses_require_ref = p.runtime_imports.__require != null and
p.symbols.items[p.runtime_imports.__require.?.inner_index].use_count_estimate > 0,
.uses_require_ref = if (p.options.bundle)
p.runtime_imports.__require != null and
p.symbols.items[p.runtime_imports.__require.?.inner_index].use_count_estimate > 0
else
p.symbols.items[p.require_ref.inner_index].use_count_estimate > 0,
.commonjs_module_exports_assigned_deoptimized = p.commonjs_module_exports_assigned_deoptimized,
.top_level_await_keyword = p.top_level_await_keyword,
.commonjs_named_exports = p.commonjs_named_exports,
Expand Down
85 changes: 27 additions & 58 deletions src/js_printer.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,7 @@ pub const SourceMapHandler = struct {
};

pub const Options = struct {
bundling: bool = false,
transform_imports: bool = true,
to_commonjs_ref: Ref = Ref.None,
to_esm_ref: Ref = Ref.None,
Expand Down Expand Up @@ -475,22 +476,6 @@ pub const Options = struct {
// const_values: Ast.ConstValuesMap = .{},
ts_enums: Ast.TsEnumsMap = .{},

// TODO: remove this
// The reason for this is:
// 1. You're bundling a React component
// 2. jsx auto imports are prepended to the list of parts
// 3. The AST modification for bundling only applies to the final part
// 4. This means that it will try to add a toplevel part which is not wrapped in the arrow function, which is an error
// TypeError: $30851277 is not a function. (In '$30851277()', '$30851277' is undefined)
// at (anonymous) (0/node_modules.server.e1b5ffcd183e9551.jsb:1463:21)
// at #init_react/jsx-dev-runtime.js (0/node_modules.server.e1b5ffcd183e9551.jsb:1309:8)
// at (esm) (0/node_modules.server.e1b5ffcd183e9551.jsb:1480:30)

// The temporary fix here is to tag a stmts ptr as the one we want to prepend to
// Then, when we're JUST about to print it, we print the body of prepend_part_value first
prepend_part_key: ?*anyopaque = null,
prepend_part_value: ?*js_ast.Part = null,

// If we're writing out a source map, this table of line start indices lets
// us do binary search on to figure out what line a given AST node came from
line_offset_tables: ?SourceMap.LineOffsetTable.List = null,
Expand Down Expand Up @@ -1836,9 +1821,7 @@ fn NewPrinter(
p.print("(");
}

if (module_type == .esm and is_bun_platform) {
p.print("import.meta.require");
} else if (p.options.require_ref) |ref| {
if (p.options.require_ref) |ref| {
p.printSymbol(ref);
} else {
p.print("require");
Expand Down Expand Up @@ -2072,7 +2055,9 @@ fn NewPrinter(
//
// This is currently only used in Bun's runtime for CommonJS modules
// referencing import.meta
if (comptime Environment.allow_assert)
//
// TODO: This assertion trips when using `import.meta` with `--format=cjs`
if (comptime Environment.isDebug)
bun.assert(p.options.module_type == .cjs);

p.printSymbol(p.options.import_meta_ref);
Expand Down Expand Up @@ -2277,9 +2262,7 @@ fn NewPrinter(
p.printSpaceBeforeIdentifier();
p.addSourceMapping(expr.loc);

if (p.options.module_type == .esm and is_bun_platform) {
p.print("import.meta.require.main");
} else if (p.options.require_ref) |require_ref| {
if (p.options.require_ref) |require_ref| {
p.printSymbol(require_ref);
p.print(".main");
} else {
Expand All @@ -2290,9 +2273,7 @@ fn NewPrinter(
p.printSpaceBeforeIdentifier();
p.addSourceMapping(expr.loc);

if (p.options.module_type == .esm and is_bun_platform) {
p.print("import.meta.require");
} else if (p.options.require_ref) |require_ref| {
if (p.options.require_ref) |require_ref| {
p.printSymbol(require_ref);
} else {
p.print("require");
Expand All @@ -2302,9 +2283,7 @@ fn NewPrinter(
p.printSpaceBeforeIdentifier();
p.addSourceMapping(expr.loc);

if (p.options.module_type == .esm and is_bun_platform) {
p.print("import.meta.require.resolve");
} else if (p.options.require_ref) |require_ref| {
if (p.options.require_ref) |require_ref| {
p.printSymbol(require_ref);
p.print(".resolve");
} else {
Expand All @@ -2331,9 +2310,7 @@ fn NewPrinter(

p.printSpaceBeforeIdentifier();

if (p.options.module_type == .esm and is_bun_platform) {
p.print("import.meta.require.resolve");
} else if (p.options.require_ref) |require_ref| {
if (p.options.require_ref) |require_ref| {
p.printSymbol(require_ref);
p.print(".resolve");
} else {
Expand Down Expand Up @@ -2550,15 +2527,6 @@ fn NewPrinter(
p.printWhitespacer(ws(" => "));

var wasPrinted = false;

// This is more efficient than creating a new Part just for the JSX auto imports when bundling
if (comptime rewrite_esm_to_cjs) {
if (@intFromPtr(p.options.prepend_part_key) > 0 and @intFromPtr(e.body.stmts.ptr) == @intFromPtr(p.options.prepend_part_key)) {
p.printTwoBlocksInOne(e.body.loc, e.body.stmts, p.options.prepend_part_value.?.stmts);
wasPrinted = true;
}
}

if (e.body.stmts.len == 1 and e.prefer_expr) {
switch (e.body.stmts[0].data) {
.s_return => {
Expand Down Expand Up @@ -5803,14 +5771,24 @@ pub fn printAst(
defer {
imported_module_ids_list = printer.imported_module_ids;
}
if (tree.prepend_part) |part| {
for (part.stmts) |stmt| {
try printer.printStmt(stmt);
if (printer.writer.getError()) {} else |err| {
return err;
}
printer.printSemicolonIfNeeded();
}

if (!opts.bundling and
tree.uses_require_ref and
tree.exports_kind == .esm and
opts.target == .bun)
{
// Hoist the `var {require}=import.meta;` declaration. Previously,
// `import.meta.require` was inlined into transpiled files, which
// meant calling `func.toString()` on a function with `require`
// would observe `import.meta.require` inside of the source code.
// Normally, Bun doesn't guarantee `Function.prototype.toString`
// will match the untranspiled source code, but in this case the new
// code is not valid outside of an ES module (eg, in `new Function`)
// https://github.com/oven-sh/bun/issues/15738#issuecomment-2574283514
//
// This is never a symbol collision because `uses_require_ref` means
// `require` must be an unbound variable.
printer.print("var {require}=import.meta;");
}

for (tree.parts.slice()) |part| {
Expand Down Expand Up @@ -6089,15 +6067,6 @@ pub fn printCommonJS(
imported_module_ids_list = printer.imported_module_ids;
}

if (tree.prepend_part) |part| {
for (part.stmts) |stmt| {
try printer.printStmt(stmt);
if (printer.writer.getError()) {} else |err| {
return err;
}
printer.printSemicolonIfNeeded();
}
}
for (tree.parts.slice()) |part| {
for (part.stmts) |stmt| {
try printer.printStmt(stmt);
Expand Down
5 changes: 0 additions & 5 deletions src/runtime.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,6 @@ pub const Runtime = struct {

trim_unused_imports: bool = false,

/// Use `import.meta.require()` instead of require()?
/// This is only supported with --target=bun
use_import_meta_require: bool = false,

/// Allow runtime usage of require(), converting `require` into `__require`
auto_polyfill_require: bool = false,

Expand Down Expand Up @@ -240,7 +236,6 @@ pub const Runtime = struct {
.dead_code_elimination,
.set_breakpoint_on_first_line,
.trim_unused_imports,
.use_import_meta_require,
.dont_bundle_twice,
.commonjs_at_runtime,
.emit_decorator_metadata,
Expand Down
Loading

0 comments on commit a3cbf97

Please sign in to comment.