Skip to content

Commit

Permalink
Fix loading react-jsxdev instead of react-jsx (#17013)
Browse files Browse the repository at this point in the history
Co-authored-by: chloe caruso <[email protected]>
  • Loading branch information
Jarred-Sumner and paperclover authored Feb 3, 2025
1 parent 7bef462 commit 7a918d2
Show file tree
Hide file tree
Showing 18 changed files with 304 additions and 27 deletions.
1 change: 1 addition & 0 deletions src/bun.js/api/JSBundler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub const JSBundler = struct {
rootdir: OwnedString = OwnedString.initEmpty(bun.default_allocator),
serve: Serve = .{},
jsx: options.JSX.Pragma = .{},
force_node_env: options.BundleOptions.ForceNodeEnv = .unspecified,
code_splitting: bool = false,
minify: Minify = .{},
no_macros: bool = false,
Expand Down
4 changes: 4 additions & 0 deletions src/bun.js/api/server/HTMLBundle.zig
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,10 @@ pub const HTMLBundleRoute = struct {

if (!server.config().development) {
config.define.put("process.env.NODE_ENV", "\"production\"") catch bun.outOfMemory();
config.jsx.development = false;
} else {
config.force_node_env = .development;
config.jsx.development = true;
}

config.source_map = .linked;
Expand Down
30 changes: 24 additions & 6 deletions src/bundler/bundle_v2.zig
Original file line number Diff line number Diff line change
Expand Up @@ -913,7 +913,14 @@ pub const BundleV2 = struct {
task.tree_shaking = this.linker.options.tree_shaking;
task.is_entry_point = is_entry_point;
task.known_target = target;
task.jsx.development = this.bundlerForTarget(target).options.jsx.development;
{
const bundler = this.bundlerForTarget(target);
task.jsx.development = switch (bundler.options.force_node_env) {
.development => true,
.production => false,
.unspecified => bundler.options.jsx.development,
};
}

// Handle onLoad plugins as entry points
if (!this.enqueueOnLoadPluginIfNeeded(task)) {
Expand Down Expand Up @@ -1753,6 +1760,9 @@ pub const BundleV2 = struct {
);
transpiler.options.env.behavior = config.env_behavior;
transpiler.options.env.prefix = config.env_prefix.slice();
if (config.force_node_env != .unspecified) {
transpiler.options.force_node_env = config.force_node_env;
}

transpiler.options.entry_points = config.entry_points.keys();
transpiler.options.jsx = config.jsx;
Expand Down Expand Up @@ -2875,7 +2885,11 @@ pub const BundleV2 = struct {
resolve_task.secondary_path_for_commonjs_interop = secondary_path_to_copy;
resolve_task.known_target = target;
resolve_task.jsx = resolve_result.jsx;
resolve_task.jsx.development = this.bundlerForTarget(target).options.jsx.development;
resolve_task.jsx.development = switch (transpiler.options.force_node_env) {
.development => true,
.production => false,
.unspecified => transpiler.options.jsx.development,
};

// Figure out the loader.
{
Expand Down Expand Up @@ -3542,7 +3556,7 @@ pub const ParseTask = struct {
};
};

const debug = Output.scoped(.ParseTask, false);
const debug = Output.scoped(.ParseTask, true);

pub fn init(resolve_result: *const _resolver.Result, source_index: Index, ctx: *BundleV2) ParseTask {
return .{
Expand Down Expand Up @@ -7545,7 +7559,7 @@ pub const LinkerContext = struct {
continue;
}

_ = this.validateTLA(id, tla_keywords, tla_checks, input_files, import_records, flags);
_ = this.validateTLA(id, tla_keywords, tla_checks, input_files, import_records, flags, import_records_list);

for (import_records) |record| {
if (!record.source_index.isValid()) {
Expand Down Expand Up @@ -11027,6 +11041,7 @@ pub const LinkerContext = struct {
input_files: []Logger.Source,
import_records: []ImportRecord,
meta_flags: []JSMeta.Flags,
ast_import_records: []bun.BabyList(ImportRecord),
) js_ast.TlaCheck {
var result_tla_check: *js_ast.TlaCheck = &tla_checks[source_index];

Expand All @@ -11038,7 +11053,7 @@ pub const LinkerContext = struct {

for (import_records, 0..) |record, import_record_index| {
if (Index.isValid(record.source_index) and (record.kind == .require or record.kind == .stmt)) {
const parent = c.validateTLA(record.source_index.get(), tla_keywords, tla_checks, input_files, import_records, meta_flags);
const parent = c.validateTLA(record.source_index.get(), tla_keywords, tla_checks, input_files, import_records, meta_flags, ast_import_records);
if (Index.isInvalid(Index.init(parent.parent))) {
continue;
}
Expand All @@ -11065,9 +11080,11 @@ pub const LinkerContext = struct {
const parent_source_index = other_source_index;

if (parent_result_tla_keyword.len > 0) {
tla_pretty_path = input_files[other_source_index].path.pretty;
const source = input_files[other_source_index];
tla_pretty_path = source.path.pretty;
notes.append(Logger.Data{
.text = std.fmt.allocPrint(c.allocator, "The top-level await in {s} is here:", .{tla_pretty_path}) catch bun.outOfMemory(),
.location = .initOrNull(&source, parent_result_tla_keyword),
}) catch bun.outOfMemory();
break;
}
Expand All @@ -11086,6 +11103,7 @@ pub const LinkerContext = struct {
input_files[parent_source_index].path.pretty,
input_files[other_source_index].path.pretty,
}) catch bun.outOfMemory(),
.location = .initOrNull(&input_files[parent_source_index], ast_import_records[parent_source_index].slice()[tla_checks[parent_source_index].import_record_index].range),
}) catch bun.outOfMemory();
}

Expand Down
8 changes: 7 additions & 1 deletion src/crash_handler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,13 @@ pub fn crashHandler(
var trace_buf: std.builtin.StackTrace = undefined;

// If a trace was not provided, compute one now
const trace = error_return_trace orelse get_backtrace: {
const trace = @as(?*std.builtin.StackTrace, if (error_return_trace) |ert|
if (ert.index > 0)
ert
else
null
else
null) orelse get_backtrace: {
trace_buf = std.builtin.StackTrace{
.index = 0,
.instruction_addresses = &addr_buf,
Expand Down
10 changes: 7 additions & 3 deletions src/js/internal/html.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// This is the file that loads when you pass a, .html entry point to Bun.
// This is the file that loads when you pass a '.html' entry point to Bun.
// It imports the entry points and initializes a server.
import type { HTMLBundle, Server } from "bun";
const initial = performance.now();
const argv = process.argv;
Expand Down Expand Up @@ -247,15 +248,18 @@ yourself with Bun.serve().
const enableANSIColors = Bun.enableANSIColors;
function printInitialMessage(isFirst: boolean) {
if (enableANSIColors) {
let topLine = `\n\x1b[1;34m\x1b[5mBun\x1b[0m \x1b[1;34mv${Bun.version}\x1b[0m`;
let topLine = `${server.development ? "\x1b[34;7m DEV \x1b[0m " : ""}\x1b[1;34m\x1b[5mBun\x1b[0m \x1b[1;34mv${Bun.version}\x1b[0m`;
if (isFirst) {
topLine += ` \x1b[2mready in\x1b[0m \x1b[1m${elapsed}\x1b[0m ms`;
}
console.log(topLine + "\n");
console.log(`\x1b[1;34m➜\x1b[0m \x1b[36m${server!.url.href}\x1b[0m`);
} else {
let topLine = `\n Bun v${Bun.version}`;
let topLine = `Bun v${Bun.version}`;
if (isFirst) {
if (server.development) {
topLine += " dev server";
}
topLine += ` ready in ${elapsed} ms`;
}
console.log(topLine + "\n");
Expand Down
5 changes: 4 additions & 1 deletion src/js_parser.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6812,7 +6812,10 @@ fn NewParser_(

if (p.lexer.jsx_pragma.jsxRuntime()) |runtime| {
if (options.JSX.RuntimeMap.get(runtime.text)) |jsx_runtime| {
p.options.jsx.runtime = jsx_runtime;
p.options.jsx.runtime = jsx_runtime.runtime;
if (jsx_runtime.development) |dev| {
p.options.jsx.development = dev;
}
} else {
// make this a warning instead of an error because we don't support "preserve" right now
try p.log.addRangeWarningFmt(p.source, runtime.range, p.allocator, "Unsupported JSX runtime: \"{s}\"", .{runtime.text});
Expand Down
41 changes: 32 additions & 9 deletions src/options.zig
Original file line number Diff line number Diff line change
Expand Up @@ -991,13 +991,18 @@ pub const ESMConditions = struct {
};

pub const JSX = struct {
pub const RuntimeMap = bun.ComptimeStringMap(JSX.Runtime, .{
.{ "classic", .classic },
.{ "automatic", .automatic },
.{ "react", .classic },
.{ "react-jsx", .automatic },
.{ "react-jsxdev", .automatic },
.{ "solid", .solid },
const RuntimeDevelopmentPair = struct {
runtime: JSX.Runtime,
development: ?bool,
};

pub const RuntimeMap = bun.ComptimeStringMap(RuntimeDevelopmentPair, .{
.{ "classic", RuntimeDevelopmentPair{ .runtime = .classic, .development = null } },
.{ "automatic", RuntimeDevelopmentPair{ .runtime = .automatic, .development = true } },
.{ "react", RuntimeDevelopmentPair{ .runtime = .classic, .development = null } },
.{ "react-jsx", RuntimeDevelopmentPair{ .runtime = .automatic, .development = true } },
.{ "react-jsxdev", RuntimeDevelopmentPair{ .runtime = .automatic, .development = true } },
.{ "solid", RuntimeDevelopmentPair{ .runtime = .solid, .development = null } },
});

pub const Pragma = struct {
Expand All @@ -1013,6 +1018,10 @@ pub const JSX = struct {
classic_import_source: string = "react",
package_name: []const u8 = "react",

/// Configuration Priority:
/// - `--define=process.env.NODE_ENV=...`
/// - `NODE_ENV=...`
/// - tsconfig.json's `compilerOptions.jsx` (`react-jsx` or `react-jsxdev`)
development: bool = true,
parse: bool = true,

Expand Down Expand Up @@ -1575,13 +1584,27 @@ pub const BundleOptions = struct {

supports_multiple_outputs: bool = true,

/// This is set by the process environment, which is used to override the
/// JSX configuration. When this is unspecified, the tsconfig.json is used
/// to determine if a development jsx-runtime is used (by going between
/// "react-jsx" or "react-jsx-dev-runtime")
force_node_env: ForceNodeEnv = .unspecified,

pub const ForceNodeEnv = enum {
unspecified,
development,
production,
};

pub fn isTest(this: *const BundleOptions) bool {
return this.rewrite_jest_for_tests;
}

pub fn setProduction(this: *BundleOptions, value: bool) void {
this.production = value;
this.jsx.development = !value;
if (this.force_node_env == .unspecified) {
this.production = value;
this.jsx.development = !value;
}
}

pub const default_unwrap_commonjs_packages = [_]string{
Expand Down
10 changes: 6 additions & 4 deletions src/resolver/tsconfig_json.zig
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,14 @@ pub const TSConfigJSON = struct {
defer allocator.free(str_lower);
_ = strings.copyLowercase(str, str_lower);
// - We don't support "preserve" yet
// - We rely on NODE_ENV for "jsx" or "jsxDEV"
// - We treat "react-jsx" and "react-jsxDEV" identically
// because it is too easy to auto-import the wrong one.
if (options.JSX.RuntimeMap.get(str_lower)) |runtime| {
result.jsx.runtime = runtime;
result.jsx.runtime = runtime.runtime;
result.jsx_flags.insert(.runtime);

if (runtime.development) |dev| {
result.jsx.development = dev;
result.jsx_flags.insert(.development);
}
}
}
}
Expand Down
23 changes: 20 additions & 3 deletions src/transpiler.zig
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ pub const Transpiler = struct {
const has_production_env = this.env.isProduction();
if (!was_production and has_production_env) {
this.options.setProduction(true);
this.resolver.opts.setProduction(true);
}

if (this.options.isTest() or this.env.isTest()) {
Expand All @@ -567,6 +568,7 @@ pub const Transpiler = struct {
this.env.loadProcess();
if (this.env.isProduction()) {
this.options.setProduction(true);
this.resolver.opts.setProduction(true);
}
},
else => {},
Expand All @@ -590,7 +592,7 @@ pub const Transpiler = struct {

try this.runEnvLoader(false);

this.options.jsx.setProduction(this.env.isProduction());
var is_production = this.env.isProduction();

js_ast.Expr.Data.Store.create();
js_ast.Stmt.Data.Store.create();
Expand All @@ -600,11 +602,26 @@ pub const Transpiler = struct {

try this.options.loadDefines(this.allocator, this.env, &this.options.env);

var is_development = false;
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 (NODE_ENV.len > 0 and NODE_ENV[0].data.value == .e_string) {
if (NODE_ENV[0].data.value.e_string.eqlComptime("production")) {
is_production = true;
} else if (NODE_ENV[0].data.value.e_string.eqlComptime("development")) {
is_development = true;
}
}
}

if (is_development) {
this.options.setProduction(false);
this.resolver.opts.setProduction(false);
this.options.force_node_env = .development;
this.resolver.opts.force_node_env = .development;
} else if (is_production) {
this.options.setProduction(true);
this.resolver.opts.setProduction(true);
}
}

pub fn resetStore(_: *const Transpiler) void {
Expand Down
37 changes: 37 additions & 0 deletions test/bundler/transpiler/jsx-dev/jsx-dev.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { renderToReadableStream } from "react-dom/server.browser";

const HelloWorld = () => {
return <div>Hello World</div>;
};

const stream = new Response(await renderToReadableStream(<HelloWorld />));

console.log(await stream.text());

if (!process.env.NO_BUILD) {
const self = await Bun.build({
entrypoints: [import.meta.path],
define: {
"process.env.NODE_ENV": JSON.stringify(process.env.CHILD_NODE_ENV),
"process.env.NO_BUILD": "1",
},
});
const code = await self.outputs[0].text();
let shouldHaveJSXDev = process.env.CHILD_NODE_ENV === "development";
let shouldHaveJSX = process.env.CHILD_NODE_ENV === "production";

if (shouldHaveJSXDev) {
if (!code.includes("jsx_dev_runtime.jsxDEV")) {
throw new Error("jsxDEV is not included");
}
}

if (shouldHaveJSX) {
if (!code.includes("jsx_runtime.jsx")) {
throw new Error("Jsx is not included");
}
}

const url = URL.createObjectURL(self.outputs[0]);
await import(url);
}
6 changes: 6 additions & 0 deletions test/bundler/transpiler/jsx-dev/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
"jsx": "react-jsxdev"
}
}
1 change: 1 addition & 0 deletions test/bundler/transpiler/jsx-production-entry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import "./jsx-production";
36 changes: 36 additions & 0 deletions test/bundler/transpiler/jsx-production.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { describe, expect, test, afterAll } from "bun:test";
import path from "path";
import { bunExe, bunEnv } from "harness";

const original_node_env = bunEnv.NODE_ENV;

// https://github.com/oven-sh/bun/issues/3768
describe("jsx", () => {
for (const node_env of ["production", "development", "test", ""]) {
for (const child_node_env of ["production", "development", "test", ""]) {
test(`react-jsxDEV parent: ${node_env} child: ${child_node_env} should work`, async () => {
bunEnv.NODE_ENV = node_env;
bunEnv.CHILD_NODE_ENV = child_node_env;
bunEnv.TSCONFIG_JSX = "react-jsxdev";
expect([path.join(import.meta.dirname, "jsx-dev", "jsx-dev.tsx")]).toRun(
"<div>Hello World</div>" + "\n" + "<div>Hello World</div>" + "\n",
);
});

test(`react-jsx parent: ${node_env} child: ${child_node_env} should work`, async () => {
bunEnv.NODE_ENV = node_env;
bunEnv.CHILD_NODE_ENV = child_node_env;
bunEnv.TSCONFIG_JSX = "react-jsx";
expect([path.join(import.meta.dirname, "jsx-production-entry.ts")]).toRun(
"<div>Hello World</div>" + "\n" + "<div>Hello World</div>" + "\n",
);
});
}
}

afterAll(() => {
bunEnv.NODE_ENV = original_node_env;
delete bunEnv.CHILD_NODE_ENV;
delete bunEnv.TSCONFIG_JSX;
});
});
Loading

0 comments on commit 7a918d2

Please sign in to comment.