wip server entry + standalone build

[nitedani]

No description provided.

[nitedani]

If you:

/examples/react-standalone-v1
pnpm dev

Logs like [vike][request(4)] HTTP response / 200 won't show.
I checked multiple things:
Using vavite in the telefunc-v1 example, on the main branch: also no logs are shown
Using this PR + vavite/node-loader in react-standalone-v1: also no logs are shown

Using this PR and vike dev in my production Vike project instead of vavite: logs are shown

At this point, I'm 99% sure, that if the PR is released, the logs will show for the user when vike dev.
I think this is a monorepo+local dependency(Vike)+Vite issue.

[nitedani]

I need some help with this part: https://github.com/vikejs/vike/pull/1342/files#diff-9e4f75824c2dea66c574aef0b7effc3ab5d1452038290c19e25fc6af458f880eR104-R177

It needs to reliably copy files and create relative symlinks in dist/node_modules.

The folder structure in dist has to look like this when using a pnpm monorepo:

This kind of works right now, but I don't know how stable it is.

References:
https://github.com/cyco130/vite-rsc/blob/main/packages/fully-react/src/fs.ts#L47
https://github.com/vercel/next.js/blob/c2ab5f704afb796a8416a51b2587535af7aabfbd/packages/next/src/build/utils.ts#L1865

[brillout]

I need some help with this part: https://github.com/vikejs/vike/pull/1342/files#diff-9e4f75824c2dea66c574aef0b7effc3ab5d1452038290c19e25fc6af458f880eR104-R177

I will have a look at it.

Logs like [vike][request(4)] HTTP response / 200 won't show.

My guess is that it's because Vike's server runtime is now processed by Vite whereas before it wasn't. Which means we need to use getGlobalObject(). I'll elaborate later why.

FYI I'm further refactoring @brillout/vite-plugin-import-build. I think I found a neat structure. I've to run a couple of IRL errands and I'll then be testing out the new structure.

[brillout]

I did a little bit of reviewing during the flight :)

• One thing that is nice about using Rollup instead of esbuild for bundling is that it alleviates Rollup the need to do the whole chunking thing which I can see to be a slow process. All-in-all it could result in a significantly faster build time. I'll push a commit (hopefully this weekend if I find the time) to this PR that replaces the renderChunk() injection, so that we can use Rollup for bundling. (This requires a small change to @brillout/vite-plugin-import-build so it's probably best I write the change.)
• I wonder if there is a way to detect native dependencies. Checking for node_modules/sharp/package.json#binary would work for Sharp, but wouldn't work for @node-rs/argon2 as its package.son only sets napi (but Sharp doesn't set napi). I wonder how other Vite frameworks handle this.
• You probably already know it, but in case I found a glitch: it seems that errors thrown by the server completely shut down the server, breaking the auto restart feature.
• I wonder whether it's possible, somehow, to catch and show a warning when the user uses a setup that is problematic with fast, e.g. when the user isn't handling the database connection properly (although it isn't clear to me yet what "properly" means, I guess there can be several strategies). I'm just thinking out loud; I don't know if there is potential to do something in that regard, and we can do such polishing later as users stumble upon issues.
• I'm thinking 'full' could be a more explicit option name than 'reliable'.
• Do we use nextTick() in order to not delay Vite's log? A comment could be nice, as it'd make reading the code slightly faster.
• I didn't review the standaloner yet.
• It was a fun read (learned a couple of neat tricks).

[nitedani]

I removed nextTick, it's not necessary.
I changed "reliable" to "full".
I made a small change: if vite.config.ts is modified, we full-reload even if "fast" is set in config. I couldn't make Vite config modifications work with "fast".

• One thing that is nice about using Rollup instead of esbuild for bundling is that it alleviates Rollup the need to do the whole chunking thing which I can see to be a slow process. All-in-all it could result in a significantly faster build time.

I think it is already as fast as it gets, but prove me wrong. Maybe I'm missing some information, but in my head it's the opposite: If we use esbuild(which is faster than Rollup), we can skip Rollup chunking, and instead let esbuild bundle all the externals that Rollup didn't.

• I wonder how other Vite frameworks handle this.

Next and Astro use nft.

• it seems that errors thrown by the server completely shut down the server, breaking the auto restart feature

I don't know where to handle this:

• We could handle this in the outer, cli process: if the exit code of the Vite process is 1, we watch for the next file-change with chokidar, and restart the Vite process.

[brillout]

• it seems that errors thrown by the server completely shut down the server, breaking the auto restart feature

I don't know where to handle this:

• We could handle this in the outer, cli process: if the exit code of the Vite process is 1, we watch for the next file-change with chokidar, and restart the Vite process.

I just pushed a commit that fixes it.

The server still shuts down if the error is thrown for the very first server start, but I think this can also be fixed by wrapping the other loadEntry() call in configureServer().

[nitedani]

I just pushed a commit that fixes it.

  app.get('*', async (req, res, next) => {
    foo

still breaks it

[nitedani]

What do you think about this? https://github.com/vikejs/vike/pull/1342/files#diff-ae1a4d37b5cb5aa071d99d1d5140bdc01eaed4f2abb7236c6f89ef4e58cd918fR68-R77

Now, if the inner user server process exits with 1, the cli doesn't exit, but waits for the next file change in config.root. This doesn't interfere with exiting using CTRL+C on my computer.

I'm not sure if hooking process.on('uncaughtException') is a good idea.
"The handler should not be used to prevent the process from crashing. Unhandled exceptions can leave your application in an undefined state"

[brillout]

The handler should not be used to prevent the process from crashing.

I'd usually agree, but I think it's fine for our use case because we aren't preventing the server from terminating: we do terminate the server, it's just that we preserve the process. I wonder if there is much of a difference between auto-reloading upon a normal change and auto-reloading upon an error: in both situations we (equally?) risk not cleaning side-effects.

What do you think about this? https://github.com/vikejs/vike/pull/1342/files#diff-ae1a4d37b5cb5aa071d99d1d5140bdc01eaed4f2abb7236c6f89ef4e58cd918fR68-R77

Now, if the inner user server process exits with 1, the cli doesn't exit, but waits for the next file change in config.root. This doesn't interfere with exiting using CTRL+C on my computer.

I like it. The downside though is that it triggers a slow complete reload instead of a quick vavite reload.

The upside of injecting a unhandledexception/uncaughtException listener to the user's server entry is that we can perform a quick vavite reload.

I guess we can go for the chockidar solution if it makes our lives non-negligibly easier.

[brillout]

// This error is caught as well. But the HTTP request hangs forever. I guess the proper way to handle this is to tell users to use an Express.js error middleware that gracefully handles errors.

I think I've been wrong: upon a quick vavite reload, we close the HTTP server, therefore terminating the forever-hanging HTTP request.

[nitedani]

I wonder if there is much of a difference between auto-reloading upon a normal change and auto-reloading upon an error: in both situations we (equally?) risk not cleaning side-effects.

we do terminate the server

Good point. Is this fine then? https://github.com/vikejs/vike/pull/1342/files#diff-a23fb21ec8d5ef2351248affc34cde56e016ee0d80d16878b29f30457af67e90R116-R122

[brillout]

Neat. I just tried and it indeed seems to work.

I'm just understanding that my mistake was to listen to the wrong process (the parent CLI process instead of child server process).

So no need to inject the listeners to the user severy entry then 👌

[brillout]

• One thing that is nice about using Rollup instead of esbuild for bundling is that it alleviates Rollup the need to do the whole chunking thing which I can see to be a slow process. All-in-all it could result in a significantly faster build time.

I think it is already as fast as it gets, but prove me wrong. Maybe I'm missing some information, but in my head it's the opposite: If we use esbuild(which is faster than Rollup), we can skip Rollup chunking, and instead let esbuild bundle all the externals that Rollup didn't.

Good point and you're right esbuild is probably a lot faster at processing node_modules/ than Rollup.

(FYI I just pushed some commits and the code generated by @brillout/vite-plugin-import-build is now part of Rollup's dependency graph, which brings a couple of advantages on the table.)

we can skip Rollup chunking

Indeed that would be nice. Chunking is quite complex and I can see Rollup spending quite some time on it.

[nitedani]

In my opinion we don't need two separate shortcuts for "fast" and "full" reload.
The "r" shortcut always uses the reload strategy set in the config. If the user know they have an issue with "fast", then they can swap to "full" in the config.

[brillout]

I think a user who uses fast may still want to do a full reload: for example, if most of the user's changes doesn't affect the database connection (fast works) while the user occasionnaly changes the database connection URI (fast doesn't work and full is required). That's why I'm inclined to think it would be neat to offer both to fast users. (Let me know if feel like this example is contrived.)

[nitedani]

I'd like to summarize the recent changes:

• added support for lazy/optional npm package imports through an inline esbuild plugin(standalone-ignore), this is needed for, for example supporting Nestjs as a server in standalone mode
• changed the config server.entry to the type string | { index:string; [name:string]: string }.
  • If it's a string, it becomes the index entry
  • If it's a mapping, setting the index is required, and other additional entries can be set for bundling server-only code. This adds support for use cases like child_process.fork("worker.js"), with this, if worker.js is added as entry alongside index, and standalone is also set to true, "worker.js" will be also bundled to a separate output file.
  • If standalone is disabled, the extra entries will be only bundled through rollup, no esbuild
• added react and react-dom to esbuild externals, this is needed to support dynamic imports in standalone mode for react, for example what react-streaming does
• changed the logic in findRollupBundleEntries - it finds multiple entries, only those, that are explicitly set in server.entry

[brillout]

Neat & makes sense. FYI brillout/react-streaming@f4da7fc which is released in react-streaming@0.3.20 so I believe we don't need to externalize React anymore.

[nitedani]

Continued in #1513