perf(remix-dev/vite): disable `server.preTransformRequests` for child compiler #8121

hi-ogawa · 2023-11-24T06:08:10Z

While investigating @remix-run/dev/server-build support for #8122, I found that static import in route file is triggering vite's warmupRequest internally, which I think is unnecessary for child compiler since remix only needs to transpile route file itself to extract exports.

https://github.com/vitejs/vite/blob/0654d1b52448db4d7a9b69aee6aad9e015481452/packages/vite/src/node/plugins/importAnalysis.ts#L621-L631

            if (
              !isDynamicImport &&
              isLocalImport &&
              config.server.preTransformRequests
            ) {
              // pre-transform known direct imports
              // These requests will also be registered in transformRequest to be awaited
              // by the deps optimizer
              const url = removeImportQuery(hmrUrl)
              server.warmupRequest(url, { ssr })
            }

testing strategy

It's a little crude, but I can see the change from the logs of DEBUG="vite:transform" vite dev.
Reproduction is here https://github.com/hi-ogawa/remix-vite-experiment-preTransformRequests which has this structure:

app/
  routes/
    route.tsx  (imports utils.tsx)
  utils.tsx

before

  vite:transform 7.25ms virtual:server-entry +0ms
  vite:transform 5.15ms /node_modules/.pnpm/@remix-run+dev@0.0.0-nightly-dda449a-20231124_@remix-run+serve@0.0.0-nightly-dda449a-20231124_mti6rk27yzoho5bnzwyf7xqtii/node_modules/@remix-run/dev/dist/config/defaults/entry.server.node.tsx +11ms
  vite:transform 24.84ms /app/routes/_index.tsx +21ms
  vite:transform 27.41ms /app/routes/route.tsx +3ms
  vite:transform 28.74ms /app/root.tsx +1ms
  vite:transform 0.83ms virtual:remix-react-proxy +1ms
  vite:transform 0.92ms virtual:remix-react-proxy +0ms
  vite:transform 12.02ms /app/utils.tsx +65ms          ////  parent compiler transforming "utils.tsx"
  vite:transform 0.17ms virtual:server-manifest +4ms
  vite:transform 1.29ms /app/utils.tsx +23ms           ////  unnecessary child compiler transform
  vite:transform 8.86ms /node_modules/.pnpm/@remix-run+dev@0.0.0-nightly-dda449a-20231124_@remix-run+serve@0.0.0-nightly-dda449a-20231124_mti6rk27yzoho5bnzwyf7xqtii/node_modules/@remix-run/dev/dist/config/defaults/entry.client.tsx +22ms
  vite:transform 0.51ms virtual:inject-hmr-runtime +27ms
  vite:transform 85.12ms /app/root.tsx +51ms
  vite:transform 85.47ms /app/routes/_index.tsx +0ms
  vite:transform 0.57ms virtual:remix-react-proxy +1ms
  vite:transform 0.09ms virtual:browser-manifest +11ms
  vite:transform 8.96ms virtual:hmr-runtime +2ms
  vite:transform 0.92ms /@vite/client +3ms
  vite:transform 0.17ms /node_modules/.pnpm/vite@5.0.2/node_modules/vite/dist/client/env.mjs +2ms
  vite:transform 1.01ms /node_modules/.vite/deps/react.js?v=65f5f71d +53ms
  vite:transform 0.72ms /node_modules/.vite/deps/react_jsx-dev-runtime.js?v=65f5f71d +1ms
  vite:transform 0.47ms /node_modules/.vite/deps/react-dom_client.js?v=65f5f71d +1ms
  vite:transform 251.50ms /node_modules/.vite/deps/@remix-run_react.js?v=65f5f71d +252ms
  vite:transform 1.08ms /node_modules/.vite/deps/chunk-R6UGEKD2.js?v=8a65ed46 +4ms
  vite:transform 9.50ms /node_modules/.vite/deps/chunk-TU7FDOKZ.js?v=8a65ed46 +11ms

after

  vite:transform 8.74ms virtual:server-entry +0ms
  vite:transform 7.34ms /node_modules/.pnpm/@remix-run+dev@0.0.0-nightly-dda449a-20231124_patch_hash=xu5ftnighaawolejqcxbv5joxa_@remix-ru_l6wv62h4ikxkky3mxm7qx6of5m/node_modules/@remix-run/dev/dist/config/defaults/entry.server.node.tsx +15ms
  vite:transform 28.89ms /app/routes/_index.tsx +24ms
  vite:transform 31.55ms /app/routes/route.tsx +2ms
  vite:transform 32.45ms /app/root.tsx +1ms
  vite:transform 0.36ms virtual:remix-react-proxy +0ms
  vite:transform 12.65ms /app/utils.tsx +67ms         ////  only parent compiler transforming "utils.tsx"
  vite:transform 0.09ms virtual:server-manifest +4ms
  vite:transform 0.72ms virtual:inject-hmr-runtime +38ms
  vite:transform 7.43ms /node_modules/.pnpm/@remix-run+dev@0.0.0-nightly-dda449a-20231124_patch_hash=xu5ftnighaawolejqcxbv5joxa_@remix-ru_l6wv62h4ikxkky3mxm7qx6of5m/node_modules/@remix-run/dev/dist/config/defaults/entry.client.tsx +6ms
  vite:transform 70.30ms /app/root.tsx +70ms
  vite:transform 70.73ms /app/routes/_index.tsx +1ms
  vite:transform 0.67ms virtual:remix-react-proxy +0ms
  vite:transform 9.39ms virtual:hmr-runtime +12ms
  vite:transform 1.54ms /@vite/client +7ms
  vite:transform 0.22ms /node_modules/.pnpm/vite@5.0.2/node_modules/vite/dist/client/env.mjs +2ms
  vite:transform 0.09ms virtual:browser-manifest +2ms
  vite:transform 0.95ms /node_modules/.vite/deps/react_jsx-dev-runtime.js?v=4fb84126 +54ms
  vite:transform 0.47ms /node_modules/.vite/deps/react-dom_client.js?v=4fb84126 +1ms
  vite:transform 0.55ms /node_modules/.vite/deps/react.js?v=4fb84126 +1ms
  vite:transform 214.98ms /node_modules/.vite/deps/@remix-run_react.js?v=4fb84126 +216ms
  vite:transform 0.80ms /node_modules/.vite/deps/chunk-R6UGEKD2.js?v=fa423614 +3ms
  vite:transform 10.03ms /node_modules/.vite/deps/chunk-TU7FDOKZ.js?v=fa423614 +12ms

… compiler

changeset-bot · 2023-11-24T06:08:14Z

🦋 Changeset detected

Latest commit: 32d5347

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
@remix-run/dev	Patch
create-remix	Patch
remix	Patch
@remix-run/architect	Patch
@remix-run/cloudflare	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/css-bundle	Patch
@remix-run/deno	Patch
@remix-run/eslint-config	Patch
@remix-run/express	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/serve	Patch
@remix-run/server-runtime	Patch
@remix-run/testing	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

pcattori · 2023-11-25T17:49:53Z

Not sure if this API is available in Vite 5. Looking at this repo, seems like server.warmup is now the intended API. But I haven't looked into it deeply, so not sure.

hi-ogawa · 2023-11-26T01:22:58Z

I think what server.warmup does it to provide a more aggressive way to trigger warmupRequest at the launch of the server.
https://github.com/vitejs/vite/blob/227d56d37fbfcd1af4b5d93182770b4e650511ee/packages/vite/src/node/server/index.ts#L776-L782

The above code snippet I copied from plugins/importAnalysis.ts is still the default heuristic/optimization done by Vite internally to trigger warmupRequest when Vite's import analysis finds static import of local files since it's likely for users to request transform of such dependency right after that. But for the case of child compiler, this heuristics is "false-positive" in a sense and I think remix can disable it to reduce unnecessary work.

It seems this preTransformRequests option was initially added for Vitest where they saw having this heuristics was negatively impacting performance vitejs/vite#6309 vitest-dev/vitest#229

…ompiler

markdalgleish

This makes sense to me. Thanks for the PR!

github-actions · 2023-12-05T20:49:34Z

🤖 Hello there,

We just published version 2.4.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

github-actions · 2023-12-13T22:26:51Z

🤖 Hello there,

We just published version 2.4.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

perf(remix-dev/vite): disable server.preTransformRequests for child…

ec3bd47

… compiler

remix-cla-bot bot added the CLA Signed label Nov 24, 2023

hi-ogawa marked this pull request as ready for review November 24, 2023 08:07

Merge branch 'dev' into perf-disable-preTransformRequests-for-child-c…

56bc35f

…ompiler

markdalgleish self-assigned this Nov 26, 2023

markdalgleish added package:dev vite labels Nov 26, 2023

add changeset

32d5347

markdalgleish approved these changes Nov 26, 2023

View reviewed changes

markdalgleish merged commit f5fe1f2 into remix-run:dev Nov 26, 2023

hi-ogawa deleted the perf-disable-preTransformRequests-for-child-compiler branch November 26, 2023 23:19

hi-ogawa mentioned this pull request Nov 28, 2023

perf(remix-dev/vite): use transformRequest for route exports anlaysis to utilize Vite's caching #8113

Closed

github-actions bot mentioned this pull request Dec 5, 2023

chore: Update version for release (pre) #8217

Merged

github-actions bot mentioned this pull request Dec 13, 2023

chore: Update version for release #8280

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

perf(remix-dev/vite): disable `server.preTransformRequests` for child compiler #8121

perf(remix-dev/vite): disable `server.preTransformRequests` for child compiler #8121

hi-ogawa commented Nov 24, 2023 •

edited

Loading

changeset-bot bot commented Nov 24, 2023 •

edited

Loading

pcattori commented Nov 25, 2023

hi-ogawa commented Nov 26, 2023 •

edited

Loading

markdalgleish left a comment

github-actions bot commented Dec 5, 2023

github-actions bot commented Dec 13, 2023

perf(remix-dev/vite): disable server.preTransformRequests for child compiler #8121

perf(remix-dev/vite): disable server.preTransformRequests for child compiler #8121

Conversation

hi-ogawa commented Nov 24, 2023 • edited Loading

testing strategy

changeset-bot bot commented Nov 24, 2023 • edited Loading

🦋 Changeset detected

pcattori commented Nov 25, 2023

hi-ogawa commented Nov 26, 2023 • edited Loading

markdalgleish left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 5, 2023

github-actions bot commented Dec 13, 2023

perf(remix-dev/vite): disable `server.preTransformRequests` for child compiler #8121

perf(remix-dev/vite): disable `server.preTransformRequests` for child compiler #8121

hi-ogawa commented Nov 24, 2023 •

edited

Loading

changeset-bot bot commented Nov 24, 2023 •

edited

Loading

hi-ogawa commented Nov 26, 2023 •

edited

Loading