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

Preloading #48

Merged
merged 5 commits into from
Jun 27, 2022
Merged

Preloading #48

merged 5 commits into from
Jun 27, 2022

Conversation

zth
Copy link
Owner

@zth zth commented Jun 24, 2022

This continues on the preloading path, and does the following:

  • Ensure all dynamically imported assets are included in our res manifest.
  • Ensure route renderer JS script tags are emitted ASAP to the stream. We'll know the client will need them as soon as we've matched the initial routes to the URL, so we can emit them right away.
  • In dev, dedupe assets between preloading on the client/server.
  • Bring back script tag based preloading client side.

Things left to fix/that aren't working:

  • Preloading via script tags in dev in the client bundle doesn't work with Vite's invalidation (appending ?t=<timestamp>), and I don't know how/if we can fix that. It's a small thing though since it's dev only, so we can revisit later.

Closes #17
Closes #46

Comment on lines +100 to +106
switch preloadAsset {
| None => ()
| Some(preloadAsset) =>
initialMatches->Belt.Array.forEach(({route}) => {
preloadAsset(#JsModule(route.name ++ "_route_renderer", route.chunk))
})
}
Copy link
Owner Author

Choose a reason for hiding this comment

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

This ensures that loading of route renderers are initiated and emitted to the client as soon as possible server side.

@zth zth requested a review from Kingdutch June 27, 2022 06:02
@zth zth merged commit b5c312f into main Jun 27, 2022
@zth zth deleted the preloading branch June 27, 2022 06:03
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 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.
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.

Enable preloading JS assets via script tags on both server and client findGeneratedModule logic duplicated
1 participant