kiliman / remix-express-vite-plugin

This package includes a Vite plugin to use in your Remix app. It configures an Express server for both development and production using TypeScript.
118 stars 6 forks source link

Incorrectly handling _root.data #19

Closed lsthornt closed 3 months ago

lsthornt commented 3 months ago

I have a route configuration that looks like the following:

routes/_app.tsx <== has middleware for all nested routes
routes/_app._index.tsx

When I am on the _app._index page (path /), and I click a link that directs to the current page, a request is fired targeting /_root.data?_routes=routes%2F_app. The middleware added to _app fails to fire.


I am still gaining familiarity with the routing internals, but my attempt at a diagnosis is as follows:

kiliman commented 3 months ago

Do you have clientLoaders or shouldRevalidate? These cause Single Fetch to call loaders differently. I haven't implemented handling those yet.

If you could create a simple repro, I'd appreciate it.

lsthornt commented 3 months ago

Yes I have shouldRevalidate = () => false; on my root Route, as it is loading ENV variables according to this pattern

Removing that changes removes the search string from the request, so it is just requesting /root.data, so that part of the original report can be considered a red herring.


Here is a minimal stackblitz to repro: https://stackblitz.com/edit/github-spaqp1-nxqwni

lsthornt commented 3 months ago

Digging further here, just posting findings as I learn...

My naïve fix would be to simply remove that conditional that forces the matches array to only the root route, but I have to assume it was added for a reason.

As is, I believe it'll cause a failure any time you use a root-level _index route, whether or not it is nested inside a pathless route as in my case

kiliman commented 3 months ago

Ugh. Thanks for looking into it. I can't say off the top of my head why I did that. It's most likely something came up when I was implementing it. Does removing that fix your use case?

I won't have time until this weekend to really get into it, so thanks for doing the grunt work.

lsthornt commented 3 months ago

Yeah, removing that conditional does resolve the issue in my app.

I've been trying to think about the hypothetical case that the conditional protects against—requesting root.data but not wanting to match to _index—but I'm not coming up with anything. Maybe you know of some special case where root.data is requested outside of the normal app navigate/fetcher flow?

kiliman commented 3 months ago

Fixed in v0.3.8

lsthornt commented 3 months ago

Having a new but similar matching issue now after upgrading to 0.3.10 today.

I've updated the Stackblitz from before: https://stackblitz.com/edit/github-spaqp1-nxqwni

In my app (not Stackblitz), when I log the matches array on route transition, I only get a single match (the root route).


This commit seems to be the culprit. req.url is used a few lines later for matching against routes.

As before, I'm unsure of what prompted the change initially, but patching in a req.url = req.originalUrl after that line does fix the bug.