fix(transpiler): remove react element inlining (#13694)

Co-authored-by: Jarred Sumner <[email protected]>
oven-sh · Sep 9, 2024 · d38f937 · d38f937
1 parent 07e4b5f
commit d38f937
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 337 deletions.
diff --git a/src/bun.js/api/JSTranspiler.zig b/src/bun.js/api/JSTranspiler.zig
@@ -526,19 +526,6 @@ fn transformOptionsFromJSC(globalObject: JSC.C.JSContextRef, temp_allocator: std
         transpiler.runtime.allow_runtime = flag;
     }
 
-    if (object.getOptional(globalThis, "jsxOptimizationInline", bool) catch return transpiler) |flag| {
-        transpiler.runtime.jsx_optimization_inline = flag;
-    }
-
-    if (object.getOptional(globalThis, "jsxOptimizationHoist", bool) catch return transpiler) |flag| {
-        transpiler.runtime.jsx_optimization_hoist = flag;
-
-        if (!transpiler.runtime.jsx_optimization_inline and transpiler.runtime.jsx_optimization_hoist) {
-            JSC.throwInvalidArguments("jsxOptimizationHoist requires jsxOptimizationInline", .{}, globalObject, exception);
-            return transpiler;
-        }
-    }
-
     if (object.getOptional(globalThis, "inline", bool) catch return transpiler) |flag| {
         transpiler.runtime.inlining = flag;
     }

diff --git a/src/bundler.zig b/src/bundler.zig
@@ -594,18 +594,6 @@ pub const Bundler = struct {
         if (this.options.define.dots.get("NODE_ENV")) |NODE_ENV| {
             if (NODE_ENV.len > 0 and NODE_ENV[0].data.value == .e_string and NODE_ENV[0].data.value.e_string.eqlComptime("production")) {
                 this.options.production = true;
-
-                if (this.options.target.isBun()) {
-                    if (strings.eqlComptime(this.options.jsx.package_name, "react")) {
-                        if (this.options.jsx_optimization_inline == null) {
-                            this.options.jsx_optimization_inline = true;
-                        }
-
-                        if (this.options.jsx_optimization_hoist == null and (this.options.jsx_optimization_inline orelse false)) {
-                            this.options.jsx_optimization_hoist = true;
-                        }
-                    }
-                }
             }
         }
     }
@@ -1425,13 +1413,7 @@ pub const Bundler = struct {
                 opts.filepath_hash_for_hmr = file_hash orelse 0;
                 opts.features.auto_import_jsx = bundler.options.auto_import_jsx;
                 opts.warn_about_unbundled_modules = target.isNotBun();
-                opts.features.jsx_optimization_inline = opts.features.allow_runtime and
-                    (bundler.options.jsx_optimization_inline orelse (target.isBun() and jsx.parse and
-                    !jsx.development)) and
-                    (jsx.runtime == .automatic or jsx.runtime == .classic) and
-                    strings.eqlComptime(jsx.import_source.production, "react/jsx-runtime");
 
-                opts.features.jsx_optimization_hoist = bundler.options.jsx_optimization_hoist orelse opts.features.jsx_optimization_inline;
                 opts.features.inject_jest_globals = this_parse.inject_jest_globals;
                 opts.features.minify_syntax = bundler.options.minify_syntax;
                 opts.features.minify_identifiers = bundler.options.minify_identifiers;

diff --git a/src/bundler/bundle_v2.zig b/src/bundler/bundle_v2.zig
@@ -2904,7 +2904,6 @@ pub const ParseTask = struct {
         opts.package_version = task.package_version;
 
         opts.features.top_level_await = true;
-        opts.features.jsx_optimization_inline = target.isBun() and (bundler.options.jsx_optimization_inline orelse !task.jsx.development);
         opts.features.auto_import_jsx = task.jsx.parse and bundler.options.auto_import_jsx;
         opts.features.trim_unused_imports = loader.isTypeScript() or (bundler.options.trim_unused_imports orelse false);
         opts.features.inlining = bundler.options.minify_syntax;

diff --git a/src/js_ast.zig b/src/js_ast.zig
@@ -271,8 +271,6 @@ pub const Flags = struct {
     pub const JSXElement = enum {
         is_key_after_spread,
         has_any_dynamic,
-        can_be_inlined,
-        can_be_hoisted,
         pub const Bitset = std.enums.EnumSet(JSXElement);
     };
 

diff --git a/src/js_parser.zig b/src/js_parser.zig
diff --git a/src/options.zig b/src/options.zig
@@ -1464,9 +1464,6 @@ pub const BundleOptions = struct {
 
     append_package_version_in_query_string: bool = false,
 
-    jsx_optimization_inline: ?bool = null,
-    jsx_optimization_hoist: ?bool = null,
-
     resolve_mode: api.Api.ResolveMode,
     tsconfig_override: ?string = null,
     target: Target = Target.browser,

diff --git a/src/runtime.zig b/src/runtime.zig
@@ -228,21 +228,6 @@ pub const Runtime = struct {
 
         set_breakpoint_on_first_line: bool = false,
 
-        /// Instead of jsx("div", {}, void 0)
-        /// ->
-        /// {
-        ///    "type": "div",
-        ///    "props": {},
-        ///    "children": [],
-        ///    key: void 0,
-        ///   $$typeof: Symbol.for("react.element"),
-        /// }
-        /// See also https://github.com/babel/babel/commit/3cad2872335e2130f2ff6335027617ebbe9b5a46
-        /// See also https://github.com/babel/babel/pull/2972
-        /// See also https://github.com/facebook/react/issues/5138
-        jsx_optimization_inline: bool = false,
-        jsx_optimization_hoist: bool = false,
-
         trim_unused_imports: bool = false,
 
         /// Use `import.meta.require()` instead of require()?

diff --git a/test/bundler/bundler_jsx.test.ts b/test/bundler/bundler_jsx.test.ts
@@ -158,8 +158,8 @@ describe("bundler", () => {
       {"$$typeof":"Symbol(jsxdev)","type":"div","props":{"className":"container","children":{"$$typeof":"Symbol(jsxdev)","type":"hello","props":{"prop":2,"children":{"$$typeof":"Symbol(jsxdev)","type":"h1","props":{"onClick":"Function:onClick","children":"hello"},"key":"undefined","source":false,"self":"undefined"}},"key":"undefined","source":false,"self":"undefined"}},"key":"undefined","source":false,"self":"undefined"}
     `,
     prodStdout: `
-      {"$$typeof":"Symbol(react.element)","type":"div","key":"null","ref":"null","props":{"children":"Hello World"},"_owner":"null"}
-      {"$$typeof":"Symbol(react.element)","type":"div","key":"null","ref":"null","props":{"className":"container","children":{"$$typeof":"Symbol(react.element)","type":"hello","key":"null","ref":"null","props":{"prop":2,"children":{"$$typeof":"Symbol(react.element)","type":"h1","key":"null","ref":"null","props":{"onClick":"Function:onClick","children":"hello"},"_owner":"null"}},"_owner":"null"}},"_owner":"null"}
+      {"$$typeof":"Symbol(jsx)","type":"div","props":{"children":"Hello World"},"key":"undefined"}
+      {"$$typeof":"Symbol(jsx)","type":"div","props":{"className":"container","children":{"$$typeof":"Symbol(jsx)","type":"hello","props":{"prop":2,"children":{"$$typeof":"Symbol(jsx)","type":"h1","props":{"onClick":"Function:onClick","children":"hello"},"key":"undefined"}},"key":"undefined"}},"key":"undefined"}
     `,
   });
   // bun does not do the production transform for fragments as good as it could be right now.
@@ -180,7 +180,7 @@ describe("bundler", () => {
       {"$$typeof":"Symbol(jsxdev)","type":"Symbol(jsxdev.fragment)","props":{"children":"Fragment"},"key":"undefined","source":false,"self":"undefined"}
     `,
     prodStdout: `
-      {"$$typeof":"Symbol(react.element)","type":"Symbol("jsx.fragment")","key":"null","ref":"null","props":{"children":"Fragment"},"_owner":"null"}
+      {"$$typeof":"Symbol(jsx)","type":"Symbol("jsx.fragment")","key":"null","ref":"null","props":{"children":"Fragment"},"_owner":"null"}
     `,
   });
   itBundledDevAndProd("jsx/ImportSource", {

diff --git a/test/bundler/transpiler/transpiler.test.js b/test/bundler/transpiler/transpiler.test.js
@@ -1411,101 +1411,6 @@ console.log(<div {...obj} key="after" />);`),
     );
   });
 
-  describe("inline JSX", () => {
-    const inliner = new Bun.Transpiler({
-      loader: "tsx",
-      define: {
-        "process.env.NODE_ENV": JSON.stringify("production"),
-        user_undefined: "undefined",
-      },
-      platform: "bun",
-      jsxOptimizationInline: true,
-      treeShaking: false,
-      inline: true,
-      deadCodeElimination: true,
-      allowBunRuntime: true,
-
-      target: "bun",
-      tsconfig: JSON.stringify({
-        compilerOptions: {
-          jsxImportSource: "react",
-        },
-      }),
-    });
-
-    it("inlines static JSX into object literals", () => {
-      expect(
-        inliner
-          .transformSync(
-            `
-export var hi = <div>{123}</div>
-export var hiWithKey = <div key="hey">{123}</div>
-export var hiWithRef = <div ref={foo}>{123}</div>
-
-export var ComponentThatChecksDefaultProps = <Hello></Hello>
-export var ComponentThatChecksDefaultPropsAndHasChildren = <Hello>my child</Hello>
-export var ComponentThatHasSpreadCausesDeopt = <Hello {...spread} />
-
-`.trim(),
-          )
-          .replaceAll("\n", "")
-          .replaceAll("  ", "")
-          .trim(),
-      ).toBe(
-        // TODO: figure out why its using jsxDEV() here. It doesn't do that with NODE_ENV=production at runtime.
-        `
-  import {
-    $$typeof as $$typeof_4ad651bb3f5de058,
-    __merge as __merge_e79ebbbc0cc1f55b
-    } from "bun:wrap";
-    export var hi = {
-      $$typeof: $$typeof_4ad651bb3f5de058,
-      type: "div",
-      key: null,
-      ref: null,
-      props: {
-        children: 123
-      },
-      _owner: null
-    }, hiWithKey = {
-      $$typeof: $$typeof_4ad651bb3f5de058,
-      type: "div",
-      key: "hey",
-      ref: null,
-      props: {
-        children: 123
-      },
-      _owner: null
-    }, hiWithRef = jsxDEV("div", {
-      ref: foo,
-      children: 123
-    }, void 0, !1, void 0, this), ComponentThatChecksDefaultProps = {
-      $$typeof: $$typeof_4ad651bb3f5de058,
-      type: Hello,
-      key: null,
-      ref: null,
-      props: Hello.defaultProps || {},
-      _owner: null
-    }, ComponentThatChecksDefaultPropsAndHasChildren = {
-      $$typeof: $$typeof_4ad651bb3f5de058,
-      type: Hello,
-      key: null,
-      ref: null,
-      props: __merge_e79ebbbc0cc1f55b({
-        children: "my child"
-      }, Hello.defaultProps),
-      _owner: null
-    }, ComponentThatHasSpreadCausesDeopt = jsxDEV(Hello, {
-      ...spread
-    }, void 0, !1, void 0, this);
-        `
-          .replaceAll("\n", "")
-          .replaceAll("  ", "")
-          .trim(),
-      );
-    });
-  });
-
   it("JSX spread children", () => {
     var bun = new Bun.Transpiler({
       loader: "jsx",