Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bake dev server tracking issue #14324

Open
48 of 91 tasks
paperclover opened this issue Oct 3, 2024 · 8 comments
Open
48 of 91 tasks

bake dev server tracking issue #14324

paperclover opened this issue Oct 3, 2024 · 8 comments
Assignees
Labels
bake:dev hot-reloading dev server for client+server applications tracking An umbrella issue for tracking big features

Comments

@paperclover
Copy link
Member

paperclover commented Oct 3, 2024

With #14258 merged, Bake can finally be tried out in canary builds (usage guide at end of issue). However, it is missing many features and testing that make it currently unsuitible for production-grade applications (See #14763).

Remember this list is not in order, and that not every item on this list is strictly a blocker. I will edit it as the plan changes.

Contributions are appreciated. Please join #bake in the Bun Discord for questions relating to the architecture of the Dev Server, as well as if you are trying it out as a user or framework author.

  • Features
    • CSS bundling. This should have a custom path that does not invoke the bundler. Waiting on Integrate CSS with bundler #14281
      • Hot reloading
      • CSR swapping styles correctly
    • Server Actions/Server Functions with "use server".
      • (TODO: write out the steps for this)
    • Source maps
    • Expose bundler options. Not all options will be exposed (for example, disabling minifySyntax is not allowed as affects DCE in an observable way)
    • Request input (currently, the Request object given to the framework entry point is fake)
      • Body streaming in
    • Response streaming (Response object is converted into a string)
    • Support the existing Plugin API
    • Bug: Mark route root as a SCB
    • Client side routing. Not as a complete built-in, but there needs to be a way for client-side framework code to request, bundle, and load routes.
    • Prerendering API
      • It should be possible for a framework to report that a route is always a static response.
        • Should support different content-types for the Accept header. Concrete use case: picking between serialized RSC payload or HTML.
      • It also should be possible to report that a route is partially pre-rendered, to store a serialized blob.
      • While the above two are not really dev-server features, it should be possible to enable them in DevServer mode by a flag.
    • Top-level await*
    • Macros*
    • separateSSRGraph: false
    • Framework should be able to specify a set of "built-in routes". For example, one may want to register POST /render to render a non-route server component.*
      • Routes with custom entry points (right now all go through framework.serverEntryPoint)
    • Server-side Debugger*
      • In VSCode extension*
    • (more)
  • API Design
    • Move from Bun.wipDevServerExpectHugeBreakingChanges to an integration in Bun.serve
    • Syntax of Route patterns (currently it is passthru to UWS)
    • How to specify route list in a way that is static for production, but reloadable for development.
    • Exposing caching rules
    • The server components API is currently designed with React in mind, but their API is not stabilized. How do we deal with their breaking changes in a reasonable fashion.
  • QoL
    • Error Modal. Currently it is a div with two CSS rules. The existing code for Bun.serve is subpar, contains too much code, and users deserves a redesigned modal. (partially contributor friendly)
      • Alternate JS payload which opens a WebSocket connection, to automatically refresh the page.
      • Prototype
      • Multiple lines of code for better context*
      • Syntax Highlighted Code*
      • Open in Editor
    • Deduplicate bundling errors on client + server files.
    • WebSocket auto-reconnection. This should run on window focus instead of hammering a closed port. (contributor friendly)
      • Visual feedback when a browser has no connection to DevServer. Must not interfere with the ability to use the web-app "offline"
    • Detect when bun install is run and reload the specific node_modules folders it changes.*
    • Visual feedback in the terminal when successful updates happen. This must be concise enough to not get in the way of other log messages; A new line per cmd+s is too much text imo.
    • Implement the client-only and server-only packages as special-cased errors.*
  • Hot-reloading
    • A file failing to resolve imports must place a dependency future files.
    • Changing a client component should not cause the a server-side re-render.
    • A file failing doesn't place a file watcher. This bug affects bun build --watch but no one has noticed???
    • When certain server components changes, it should be possible to update them on the client without the browser refreshing. This requires framework integration.
    • (at least a dozen bugs where hot-reloading does not do the right action)
    • TODO: Repair live bindings
    • TODO: check if this local is immediately assigned require() if so, we will instrument it with hot module reloading. Left unimplemented
    • Any changes to server component manifests are not propagated
    • Keep track of hot-reloading "boundaries". When the entry point is modified
      • Implement module.hot (webpack)
      • Implement import.meta.hot (vite)
    • Deleting a file must remove it from it's IncrementalGraph and rebuild direct dependents. In turn, that should place a directory watcher for when the file is restored.
    • When Cmd+R is pressed, it must not serve a stale bundle. The design for this to be fast is already present, but bundle re-assembly is not yet implemented.
    • When import resolution stops SSR from working, it should cache that failure until a hot reload. This happens currently because server eval must not cause an error, so instead it is being deferred way too late.
    • A build containing failing and passing files should not discard the failed files.
  • Built in React preset
    • Client-side navigation with <a> tags. (contributor friendly)
    • Stop using react-server-dom-webpack, get react-server-dom-bun
    • Implement server functions
    • (more)
  • Chores
    • Formal documentation
    • Bun.build (with target browser) produces nodejs target if the dependency is imported in the same script that runs Bun.build #14253
    • Consider restoring the separate bundler thread if server code evaluation is blocking too long. I have noticed that spamming HMR events makes DevServer lag behind. This is either due to VSCode buffering saves together, or the spam of the event loop causes websocket backpressure. I have not fully explored the options, but it is worth investigating if the bundling could be done on the watcher thread. If not, then a third thread is used. A mutex would be needed for the client-side asset files, but not for the server side code. Would be bad. Instead, make bundling asyncronous. This will fix a crash.
    • Rewire the watcher integration so it does not (1) have a limit of 8 files per batch or (2) re-allocate each path every time.
    • Missing some import.meta.env defines that we miss to match vite (contributor friendly)
    • Remove the need for the __webpack_require__ polyfill (get react-server-dom-bun)
  • Missing bundling errors:
    • Detect incorrect usages of "use server" and "use client"*
    • Warn against usages of "use server" and "use client" when server components are disabled.*
    • Ban indirect references to the Bun global. This restriction is being imposed so that bun build --compile can one day generate much smaller production binaries (e.g. someone not using Bun.build/Bun.Transpiler in their app can drop the entire bundler from the binary). Since Bake is new, we can add this restriction here but not in the runtime, and it is not a breaking change.
  • React Refresh Transform
    • Expr.Data.writeToHasher is incomplete (contributor friendly)
  • Calls to todoPanic
    • generation for 'bun:bake/client'
    • hot-module-reloading instrumentation for 'export { ... } from'
    • hot-module-reloading instrumentation for 'export * from'
    • non-framework provided entry-point not supporting
    • support non-server components build*
    • handle listen failure
  • Windows support (might "just work", but I have not tested; I feel like something will come up)

I will update this as I think of more. These are just the steps that come to mind. Additionally, I will open another issue containing the plan for production builds, though I think some of this work waits on the API design.

*does not block releasing 1.2 imo

Reliability

There are currently 3 unit tests in total for Bake. Given the size and scope of the dev server alone, much more testing and tooling is needed.

  • Additions to ./test/bundler
    • Testing the dev bundler format (--format=internal_kit_dev), initial payload
    • Testing React Fast Refresh transform (--react-fast-refresh)
    • Testing React Server Components transform (--server-components)
  • Test bake.DevServer.IncrementalGraph. This is done using a wrapper over the dev server, using the WebSocket protocol to bind custom actions.
    • Include bindings to assert that certain files are being watched / not being watched. For example, a directory watch is added
  • Some end-to-end tests which test that things such as the rendering of the error modal
  • Hot-reloading stress tests where thousands of files are constantly being saved. Bun must not crash. After <2 seconds of the process idle, the hot reloader should be smart enough to have the browser in the correct state without further user input.
  • Test for file descriptor leaks
  • Test for memory leaks
  • Consider a hot-reloading fuzzer. (outside help appreciated)

Performance

While I have done little testing on the performance, I am confident that large projects will show decent reloading speed, but there is significant cost placed on the first bundle task. I'm sure a lot of work can be done to profile and micro-optimize things, but here is a list of broader changes to the design:

  • Earlier stated idea about reviving BundleThread Decided to not do.
  • Eagerly bundling but not evaluating route contents
  • Perform IncrementalGraph garbage collection (1) at all, (2) while a browser tab is focused and source files are unlikely to be saved
  • spicy idea. Serialize IncrementalGraph to disk, so that closing and re-opening the dev server does not have to rebundle anything. Care must be taken to not be vulnerable to cache poisoning. This might not compelling, especially on smaller projects where the total time to build all files is under a second. I am only listing this point because I realized it was easy to write.

Open-ended performance tasks:

  • Test large projects as all of my claims for good performance is theory and speculation. There are surely implementation mistakes.
  • Write a hot-reloading benchmark and to objectively measure the performance impact of changes to the code.

Informal Usage Guide

TODO: the usage guide is out of date and has been removed

@paperclover paperclover added tracking An umbrella issue for tracking big features bake:dev hot-reloading dev server for client+server applications labels Oct 3, 2024
@paperclover paperclover self-assigned this Oct 3, 2024
@bndkt
Copy link

bndkt commented Oct 5, 2024

@paperdave I just discovered this tracking issue and am really excited about it! I also saw react-server-dom-bun mentioned there. I had started working on this as part of building https://github.com/bndkt/kotekan (which I did abandon in favor of another approach which still heavily involves Bun and which will be at least partially obsolete once the things mentioned in this issue land). I'm happy to transfer https://www.npmjs.com/package/react-server-dom-bun to you, just ping me on https://x.com/bndkt.

@paperclover
Copy link
Member Author

I'm happy to transfer [react-server-dom-bun] to you

Great. I had spotted this package existing and I was a little worried on the difficulty of getting the package. I don't fully know what the plan will be, but I think the react project itself publishes the -webpack and -turbopack versions of the react-server.

So far, I got server components to work using react-server-dom-webpack using the browser builds of both server and target (so i could use renderToReadableStream), but those have a dependency on __webpack_require__. I currently work around this by just defining webpack_require

// TODO: Remove this after `react-server-dom-bun` is uploaded
globalThis.__webpack_require__ = (id: string) => {
  if (side == "server" && config.separateSSRGraph && !id.startsWith("ssr:")) {
    return loadModule("ssr:" + id, LoadModuleType.UserDynamic).exports;
  } else {
    return loadModule(id, LoadModuleType.UserDynamic).exports;
  }
};

on the server, i noticed that react sort of just ignores the manifest when using the browser build. when using the node.js server build, it uses dynamic import for that module, which works a bit better (but no readable stream api).

so the bun version would basically work the same, but i would probably hold off doing it for a little bit longer in case some more bundler changes happen and there is a more optimal way to write the integration.

@paperclover
Copy link
Member Author

paperclover commented Oct 20, 2024

was messing around with the state of things, mainly writing own framework code. my notes

  • assets crash
  • import {x as y} emits ns.y instead of ns.x
  • bun has no good mechanism to properly override the jsx import
  • weird situation with a server file resolving to an ssr one, but it emitting ssr: and it fails to load at runtime.

@paperclover paperclover changed the title dev server tracking issue bake dev server tracking issue Oct 23, 2024
@huseeiin
Copy link
Contributor

huseeiin commented Oct 23, 2024

on server functions, runtime type safety is tricky.

const func = (name: string) => {
  "use server";
  console.log({ name }); // could be number
};

<button onClick={func}>Click</button>;

why not create a hono/elysia-like rpc from fs routes located in routes/api/**/* where a file there looks like this instead:

import z from "zod";

export const POST = async ({ request }: { request: Request }) => {
  z.object({ name: z.string() }).safeParse(await request.json());
};

this way you have better access to context, like the Request, and more runtime validation that is beyond just ts intellisense

@paperclover
Copy link
Member Author

why not [...]

frameworks will likely be able to customize/implement the serialization and validation layer.

while i haven't fully read the react rfcs and implementations, i think it a framework could provide zod integration via a wrapped function:

const func = validatedAction(z.object({ name: z.string() }), ({ name }) => {
  "use server";
  // perform action
  return true;
});

if the bundler just annotates the arrow function with a unique id (which is something that the framework can control), validatedAction (which the framework also controls) can forward that.

no guarentee on this thought though.

wonder if we could even omit "use server" in this case and make the action entirely defined via validatedAction

@huseeiin
Copy link
Contributor

huseeiin commented Oct 23, 2024

i got some questions. where is the code that gets the error from zod and returns it stored? how do you consume that error? is the error typed? can you use something other than zod?

@paperclover
Copy link
Member Author

these are all up to the decision of the framework; the underlying api/bundler doesn't care.

@paperclover
Copy link
Member Author

paperclover commented Nov 21, 2024

css dev server related:

  • all css errors point into invalid memory. solved temporarily by just not printing the css errors. not good quality imo
  • an asset file that does not exist does not cause a resolution error (consider content: url(./nothing.svg) emits content:
  • an asset file that does exist emits the absolute filesystem path. very close, but needs to turn into an alias url that dev server can map into that path
    • asset files being watched as a part of the graph, indexed as dependencies of css/js files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bake:dev hot-reloading dev server for client+server applications tracking An umbrella issue for tracking big features
Projects
None yet
Development

No branches or pull requests

3 participants