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

ReScriptify Server and implement asset loading #51

Merged
merged 10 commits into from Jun 29, 2022

Conversation

Kingdutch
Copy link
Collaborator

The individual commits contain most of the explanation of what is happening.

The goal was to turn server.mjs into a Server.res file so that we have ReScript across the board. In so doing I also wanted to limit the needed API surface for EntryServer.res, making the thing that handles "Setting up a webserver" and "Turning a request into a React response" two separate things. Doing so should make it easier to support different webservers such as Fastify or different environments such as Cloudflare workers. It also more clearly encapsulates what is scaffolding and what is actually React related. This should make moving things around in the router easier too.

While doing so asset preloading was something that was most spread out and as such got quite a bit of attention. As a result we now have asset preloading working (although not yet for the assets of the initial route match, only assets emitted by React) in both development and production for both SSR and CSR.

@Kingdutch Kingdutch added the enhancement New feature or request label Jun 28, 2022
@Kingdutch Kingdutch requested a review from zth June 28, 2022 09:30
This reverts commit b5c312f, reversing
changes made to c8c0f27.

This removes changes to the plugin and does not yet add the route
preLoading code. It leaves in place those things that were later used
for work done in PRs #49 and #50.
…n-source"

This reverts commit c8c0f27, reversing
changes made to 1631b15.
Copy link
Owner

@zth zth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great overall, nice work! I have a few questions/thoughts that I'm thinking you can probably address quite quickly. Good work!

RescriptRelayVitePlugin.mjs Outdated Show resolved Hide resolved
RescriptRelayVitePlugin.mjs Show resolved Hide resolved
RescriptRelayVitePlugin.mjs Outdated Show resolved Hide resolved
RescriptRelayVitePlugin.mjs Show resolved Hide resolved
RescriptRelayVitePlugin.mjs Show resolved Hide resolved
router/RelayRouter__Types.res Outdated Show resolved Hide resolved
router/RelayRouter__Utils.res Outdated Show resolved Hide resolved
router/RelayRouter__Utils.resi Outdated Show resolved Hide resolved
test/RescriptRelayVitePlugin.test.mjs Outdated Show resolved Hide resolved
src/EntryServer.res Show resolved Hide resolved
This is the start of converting the server.mjs to ReScript which
requires the addition of some bindings for server side code.

The main goal of these changes is to enable moving the React rendering
side and its effects (emission of assets) entirely into the EntryServer.

This should make it possible to create a pattern around renderTo*Stream
where utilities (e.g. the `PreloadInsertingStream` and others) can be
wired up to work regardless of the server environment.

Typing the original asset to script conversion code was too difficult to
complete in a day, so to ensure this commit brings code into a
buildable state that can be shared and interated upon.

There are two important TODO's that have significant changes from the
working code in another project:

1. Vite scripts are rewritten to script tags, in the other project these
are passed in as `head` in development mode (and head is empty in
production) but this makes EntryServer's default export bigger and
requires propagating this development snippet through the tree, which
feels wrong.

2. We write HTML to the stream manually. My suggestion is to make the
entire document part of the HTML tree. See the explanation and
motivation on line 73-100 in EntryServer.res.
Vite already helps us transform dynamic imports for our ReScript modules
(for which we don't know the name of the .mjs or .bs.js or .cjs file
when writing the ReScript code) to imports of the proper JS files.

However, in some places we also want to reference the file directly, for
example to be able to generate a `<script>` tag ourselves without use of
`import`. Up to now this relied on a runtime inspection of the ReScript
build artifacts.

This commit introduces a special non-existent function in ReScript which
serves as a marker for a newly introduced Vite plugin code transform.
This allows the code transform to replace the function invokation with
the literal string pointing to the absolute path of the source file.

This allows runtime code to use the ssrManifest generated by Vite to
turn this file-path into an asset chunk during production or to
transform it into a path relative to the Vite dev-server working
directory during development.
This moves from the marker function to a very specific object property
name. This follows what was done in:
- zth/rescript-relay#369
- #47
- #48

This reimplements the changes of those last two R3 PRs on top of the
work done to turn server.mjs into ReScript code. A few changes are made
with respect to the original PRs.

*Vite Plugin*
- I did not alter the user’s config. The rollup option not working with
  the `SSR` option is not caused by our plugin, so it shouldn’t be fixed
  in our plugin. It’s a user configuration error. Altering the config
  without the user knowing may confuse them.
- Removed `production` check in `transform` this allows using the Vite
  generated `ssr-manifest.json`
- Removed the `Rescript` check in `transform`, our regex should be fast
  enough that this doesn’t matter and it ensures our plugin works in
  codebases or pipelines that strip this header.
- Removed tracking of `didReplace` since `magic-string` does this for us.

*EntryServer.res*
- Omitted asset emitting change for RelayRouter since the function has a
  different shape than RelaySSRUtils.AssetRegisterer. This was because
 `eagerPreloadFn` was not available in the function the router got.
  Ideally when we bring this back we only require the consuming developer
  to create a single preload handler on top of our transformation stream.

*package.json*
- It looks like the introduction of SSR in production mode has also exposed
  the bug in Relay’s faulty exports (or ReScript/Vite’s handling of them)
  here.
- Upgrade `history` to 5.3.0 which is required for ESM support.
This was used for some ReScript file loading during dev SSR but that
functionality is now entirely contained within the Vite plugin.
Re-introduces the change from 1de74d8
which works now that we have a way to reliably write asset names of our
chunks into the compiled code.
There's no reason to await the route renderers of our initial routes now
that suspense and preloading works properly.
We use the plugin to work around a bug in Vite 2 which has been fixed in
Vite 3 that breaks the SSR build if the manualChunks Rollup config is
used.

See vitejs/vite#8836
The __$rescriptChunkName__ is a non-public identifier used to bridge the gap
between the ReScript and JavaScript world. It's public API is `chunk` within
ReScript and it's not intended to be used from a non-ReScript codebase.
@Kingdutch Kingdutch merged commit 5ac6e16 into main Jun 29, 2022
@Kingdutch Kingdutch deleted the feature/rescriptify-server branch June 29, 2022 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants