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

Vite dev: inconsistent behavior when adding/removing loader export #8114

Closed
hi-ogawa opened this issue Nov 23, 2023 · 1 comment
Closed

Vite dev: inconsistent behavior when adding/removing loader export #8114

hi-ogawa opened this issue Nov 23, 2023 · 1 comment

Comments

@hi-ogawa
Copy link
Contributor

hi-ogawa commented Nov 23, 2023

Reproduction

I created a reproduction here https://stackblitz.com/edit/remix-run-remix-abgccv?file=app%2Froutes%2Ftest.tsx

The following steps is for the scenario of removing loader, but adding loader also seems to have some odd behavior.

steps

  • visit "/"
  • comment out export const loader = ... in app/routes/test.tsx
  • visiting "/test" will trigger an error
    (actually this step might not always cause this error, which is kind of expected since HMR mutates route manifest on client directly?)
  • refreshing "/test" will succeed it. but navigating back to "/test" will trigger the same error.

error log

Error: You made a GET request to "/test" but did not provide a `loader` for route "routes/test", so there is no way to handle the request.
    at getInternalRouterError (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/router/dist/router.cjs.js:4437:59)
    at loadRouteData (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/router/dist/router.cjs.js:3555:13)
    at queryImpl (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/router/dist/router.cjs.js:3420:26)
    at Object.queryRoute (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/router/dist/router.cjs.js:3386:24)
    at handleDataRequestRR (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/server-runtime/dist/server.js:107:40)
    at requestHandler (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/server-runtime/dist/server.js:81:24)
    at eval (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/dev/dist/vite/node/adapter.js:191:26)
    at eval (/home/projects/remix-run-remix-abgccv/node_modules/@remix-run/dev/dist/vite/plugin.js:557:21)
    at call (/home/projects/remix-run-remix-abgccv/node_modules/vite/dist/node/chunks/dep-ErEj4WmL.js:42907:7)
    at next (/home/projects/remix-run-remix-abgccv/node_modules/vite/dist/node/chunks/dep-ErEj4WmL.js:42851:5)
screenshot

image

notes

From quick debugging, I'm suspecting there's an issue with virtual module (route manifest?) invalidation. Currently virtual modules are not invalidated by simple file content change, so for example, client might be using stale hasLoader flag even after browser reload.
To fix this, either restart the server or triggering invalidation by adding new dummy route file is currently necessary.

To mitigate this issue, my tentative ideas are following:

  • invalidate virtual modules on any route file change (add/remove/change) via ViteDevServer.watcher instead of checking resolvePluginConfig on request time
  • however this increased invalidation would degrade experience due to currently heavy getDevManifest/getRouteModuleExports, but remix could optimize it by making use of vite's transform caching (cf. #8111, #8113)

Or as an alternative simpler fix, it might be possible to check the diff of route exports during handleHotUpdate. If there is a different export, then route manifest virtual module should be invalidated.

System Info

(On stackblitz)

 System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.18.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.10.5 - /usr/local/bin/pnpm
  npmPackages:
    @remix-run/dev: * => 2.3.1 
    @remix-run/eslint-config: * => 2.3.1 
    @remix-run/node: * => 2.3.1 
    @remix-run/react: * => 2.3.1 
    @remix-run/serve: * => 2.3.1 
    vite: ^5.0.0 => 5.0.2

Used Package Manager

npm

Expected Behavior

No error

Actual Behavior

Error due to client calling loader when it's not defined.
The same issue persists after browser reload. Server restart is required to get back to healthy state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants