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

Experiment with module urls in source #47

Merged
merged 3 commits into from
Jun 24, 2022

Conversation

zth
Copy link
Owner

@zth zth commented Jun 24, 2022

Needs to be paired with: https://github.com/zth/rescript-relay/tree/experiment-with-vite-plugin
Solves the first part of #46 - actually inlining the bundle source urls.

This sets us up to enable "proper" preloading of code assets via script tags. This is slightly difficult, because in order to preload on the client with script tags, we'll need to know what bundle src url to load, and we can't know that until the full build has finished in production mode (and we have all the bundle URLs + what they contain, so we can map out what to load when). This PR handles the various cases we need in the following way:

Server, dev
In dev mode we just need to know what ReScript module we want to preload. Since Vite does no bundling in dev, we'll want to preload the full path to the ReScript module file. This PR will give us the full ReScript module name, which we can then do a lookup for the source file name and location via the same mechanism we use to resolve @rescriptModule/ModuleName references.

TLDR; In server dev, nothing is changed, the output (which is already enough for our purposes) is preserved.

Server, prod
Same goes for the server in prod - we just need to know the ReScript module name so we can look up the source file for that module. However, the lookup mechanism will be different in prod since the ReScript code will now be bundled, and we'll need to know which bundle the module is located in. For this, we emit a simple dict manifest that maps ReScript module names to which bundle it's in. We'll use that manifest in prod to look up what preload script tag to emit into the stream.

TLDR; Nothing is changed in the source here either, although we'll need to use the emitted client manifest.

Client, dev
Like in server dev, we need to know what exact file to load for any given ReScript module name. We handle this by inlining the full file path into the generated JS files where necessary. The client can then emit preload script tags pointing to the exact file for the target ReScript module, and Vite will handle the rest. This works because Vite does no bundling in dev, it only works with full file paths of raw JS files.

TLDR; We inline the file path of ReScript modules where necessary, to preload.

Client, prod
This is the trickiest one, because here we'll need to inline the final resulting bundle location for any given ReScript module. The emitted manifest already has this information, but we want to avoid loading the manifest in the client since that will potentially be very large if you have a larger app. We inline the final bundle location of any given ReScript module as a post bundling step in Rollup, where we have all the information needed to replace our placeholders with actual bundle JS file paths. We also emit a dedicated ReScript module manifest that the server will leverage to emit proper preload script tags as we stream.

TLDR; We inline the final bundle JS file location, and we emit a dedicated ReScript module manifest for all modules -> bundle location, that the server will use to emit preload script tags.

@zth zth requested a review from Kingdutch June 24, 2022 06:54
@zth zth merged commit c8c0f27 into main Jun 24, 2022
@zth zth deleted the experiment-with-module-urls-in-source branch June 24, 2022 06:58
Kingdutch added a commit that referenced this pull request Jun 28, 2022
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.
Kingdutch added a commit that referenced this pull request Jun 28, 2022
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.
Kingdutch added a commit that referenced this pull request Jun 28, 2022
…n-source"

This reverts commit c8c0f27, reversing
changes made to 1631b15.
Kingdutch added a commit that referenced this pull request Jun 28, 2022
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.
Kingdutch added a commit that referenced this pull request Jun 28, 2022
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.
Kingdutch added a commit that referenced this pull request Jun 28, 2022
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant