bake: csr, streaming ssr, serve integration, safer jsvalue functions, &more #14900
Conversation
|
❌ @paperdave, your commit 2467e2e has 2 failures in test/cli/run/require-cache.test.ts - 1 failing on 🪟 x64test/js/bun/http/serve.test.ts - 1 failing on 🍎 aarch64test/js/bun/http/serve.test.ts - 1 failing on 🍎 aarch64 |
paperclover
changed the title
| bun.Output.errGeneric("Cannot use client-side --target={s} with --server-components", .{@tagName(target)}); | ||
| Global.crash(); | ||
| } | ||
| if (bun.FeatureFlags.bake and args.flag("--server-components")) { |
There was a problem hiding this comment.
no more comptime? it's just a few branches but we can easily eliminate them with the enclosing comptime FeatureFlags.bake
There was a problem hiding this comment.
bun.FeatureFlags.bake is a compile time variable. i know this because it is a const defined outside of a function. Zig has a semantic guarantee to inline and eliminate this branch, so this is zero branches for the same logic, and presented more concisely. i know this guarantee is always going to hold true because if it did not, many places in the codebase would fail to compile.
additionally, i wrote this code before the version of this was added on main.
| config.deinit(); | ||
| return e; | ||
| }; | ||
|
|
||
| if (globalObject.hasException()) { |
There was a problem hiding this comment.
can we delete this check? does the error thrown from ServerType.init cover it?
There was a problem hiding this comment.
i will audit locations like this soon in my project
src/bun.js/node/node_fs.zig
Outdated
| @@ -2501,7 +2501,7 @@ pub const Arguments = struct { | |||
| arguments.eat(); | |||
|
|
|||
| if (val.isObject()) { | |||
| if (val.getOptional(ctx, "recursive", bool) catch { | |||
| if (val.getBooleanStrict(ctx.ptr(), "recursive") catch { | |||
| }; | ||
|
|
||
| for (options.framework.file_system_router_types, 0..) |fsr, i| { | ||
| const joined_root = bun.path.joinAbs(cwd, .auto, fsr.root); |
There was a problem hiding this comment.
the buffer used by joinAbs is guaranteed to not be used by readDirInfoIgnoreError?
| @@ -123,6 +225,12 @@ pub const Framework = struct { | |||
| // f.resolveHelper(client, &sc.client_runtime_import, &had_errors); | |||
| } | |||
|
|
|||
| for (clone.file_system_router_types) |*fsr| { | |||
| fsr.root = bun.path.joinAbs(server.fs.top_level_dir, .auto, fsr.root); | |||
There was a problem hiding this comment.
The use of this shared buffer is a little suspicious to me
| var i_2: usize = 0; | ||
| const extensions = try arena.alloc([]const u8, len); | ||
| while (it_2.next()) |array_item| : (i_2 += 1) { | ||
| extensions[i_2] = refs.track(try array_item.toSlice2(global, arena)); |
There was a problem hiding this comment.
Do you think it would be worth checking if any of the strings in the array are *, like the isString() case above?
… &more (#14900) Co-authored-by: paperdave <[email protected]> Co-authored-by: Jarred Sumner <[email protected]>
… &more (#14900) Co-authored-by: paperdave <[email protected]> Co-authored-by: Jarred Sumner <[email protected]>
What does this PR do?
Despite the
expectHugeBreakingChangeswarning gone, this API is still under revision and will recieve more huge breaking changes. To use DevServer past this commit, useexport default