From ffe447ba5a48b45561eb8faadfcd153814abe5eb Mon Sep 17 00:00:00 2001 From: dave caruso Date: Sat, 2 Dec 2023 00:42:34 -0800 Subject: [PATCH] fix(standalone): runtime dynamic imports now transpile correctly. (#7404) * fix issue in compile * add tests * [autofix.ci] apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- src/bun.js/javascript.zig | 2 +- src/bun_js.zig | 1 + src/resolver/resolver.zig | 15 +++++++ test/bundler/bundler_compile.test.ts | 66 ++++++++++++++++++++++++++++ test/bundler/expectBundled.ts | 3 ++ 5 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/bun.js/javascript.zig b/src/bun.js/javascript.zig index ab24ef08e7..70642087a9 100644 --- a/src/bun.js/javascript.zig +++ b/src/bun.js/javascript.zig @@ -1231,7 +1231,7 @@ pub const VirtualMachine = struct { pub const Options = struct { allocator: std.mem.Allocator, - args: Api.TransformOptions = std.mem.zeroes(Api.TransformOptions), + args: Api.TransformOptions, log: ?*logger.Log = null, env_loader: ?*DotEnv.Loader = null, store_fd: bool = false, diff --git a/src/bun_js.zig b/src/bun_js.zig index fd74d73ddc..40fe5d146e 100644 --- a/src/bun_js.zig +++ b/src/bun_js.zig @@ -62,6 +62,7 @@ pub const Run = struct { .vm = try VirtualMachine.initWithModuleGraph(.{ .allocator = arena.allocator(), .log = ctx.log, + .args = ctx.args, .graph = graph_ptr, }), .arena = arena, diff --git a/src/resolver/resolver.zig b/src/resolver/resolver.zig index 4896e3460a..e1cb0a7b2e 100644 --- a/src/resolver/resolver.zig +++ b/src/resolver/resolver.zig @@ -555,6 +555,21 @@ pub const Resolver = struct { } pub inline fn usePackageManager(self: *const ThisResolver) bool { + // TODO(@paperdave): make this configurable. the rationale for disabling + // auto-install in standalone mode is that such executable must either: + // + // - bundle the dependency itself. dynamic `require`/`import` could be + // changed to bundle potential dependencies specified in package.json + // + // - want to load the user's node_modules, which is what currently happens. + // + // auto install, as of writing, is also quite buggy and untested, it always + // installs the latest version regardless of a user's package.json or specifier. + // in addition to being not fully stable, it is completely unexpected to invoke + // a package manager after bundling an executable. if enough people run into + // this, we could implement point 1 + if (self.standalone_module_graph) |_| return false; + return self.opts.global_cache.isEnabled(); } diff --git a/test/bundler/bundler_compile.test.ts b/test/bundler/bundler_compile.test.ts index cb1fc45b28..d52cc84a63 100644 --- a/test/bundler/bundler_compile.test.ts +++ b/test/bundler/bundler_compile.test.ts @@ -86,4 +86,70 @@ describe("bundler", () => { }, compile: true, }); + itBundled("compile/DynamicRequire", { + files: { + "/entry.tsx": /* tsx */ ` + const req = (x) => require(x); + const y = req('commonjs'); + const z = req('esm').default; + console.log(JSON.stringify([w, x, y, z])); + module.exports = null; + `, + "/node_modules/commonjs/index.js": "throw new Error('Must be runtime import.')", + "/node_modules/esm/index.js": "throw new Error('Must be runtime import.')", + "/node_modules/other/index.js": "throw new Error('Must be runtime import.')", + "/node_modules/other-esm/index.js": "throw new Error('Must be runtime import.')", + }, + runtimeFiles: { + "/node_modules/commonjs/index.js": "module.exports = 2; require('other');", + "/node_modules/esm/index.js": "import 'other-esm'; export default 3;", + "/node_modules/other/index.js": "globalThis.x = 1;", + "/node_modules/other-esm/index.js": "globalThis.w = 0;", + }, + run: { + stdout: "[0,1,2,3]", + setCwd: true, + }, + compile: true, + }); + itBundled("compile/DynamicImport", { + files: { + "/entry.tsx": /* tsx */ ` + import 'static'; + const imp = (x) => import(x).then(x => x.default); + const y = await imp('commonjs'); + const z = await imp('esm'); + console.log(JSON.stringify([w, x, y, z])); + `, + "/node_modules/static/index.js": "'use strict';", + "/node_modules/commonjs/index.js": "throw new Error('Must be runtime import.')", + "/node_modules/esm/index.js": "throw new Error('Must be runtime import.')", + "/node_modules/other/index.js": "throw new Error('Must be runtime import.')", + "/node_modules/other-esm/index.js": "throw new Error('Must be runtime import.')", + }, + runtimeFiles: { + "/node_modules/commonjs/index.js": "module.exports = 2; require('other');", + "/node_modules/esm/index.js": "import 'other-esm'; export default 3;", + "/node_modules/other/index.js": "globalThis.x = 1;", + "/node_modules/other-esm/index.js": "globalThis.w = 0;", + }, + run: { + stdout: "[0,1,2,3]", + setCwd: true, + }, + compile: true, + }); + // see comment in `usePackageManager` for why this is a test + itBundled("compile/NoAutoInstall", { + files: { + "/entry.tsx": /* tsx */ ` + const req = (x) => require(x); + req('express'); + `, + }, + run: { + error: 'Cannot find package "express"', + }, + compile: true, + }); }); diff --git a/test/bundler/expectBundled.ts b/test/bundler/expectBundled.ts index 7486deb288..ad09d87bc2 100644 --- a/test/bundler/expectBundled.ts +++ b/test/bundler/expectBundled.ts @@ -269,6 +269,8 @@ export interface BundlerTestRunOptions { errorLineMatch?: RegExp; runtime?: "bun" | "node"; + + setCwd?: boolean; } /** given when you do itBundled('id', (this object) => BundlerTestInput) */ @@ -1218,6 +1220,7 @@ for (const [key, blob] of build.outputs) { IS_TEST_RUNNER: "1", }, stdio: ["ignore", "pipe", "pipe"], + cwd: run.setCwd ? root : undefined, }); if (run.error) {