Remove middleware depth restrictions #13172
Conversation
🦋 Changeset detectedLatest commit: c689c79 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
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 |
| @@ -2101,7 +2101,7 @@ test.describe("Middleware", () => { | |||
|
|
|||
| (await page.$('a[href="/a/b"]'))?.click(); | |||
| await page.waitForSelector("[data-b]"); | |||
| expect(await page.locator("[data-a]").textContent()).toBe("A: a"); | |||
| expect(await page.locator("[data-a]").textContent()).toBe("A: a,b"); | |||
There was a problem hiding this comment.
@jacob-ebey This might be a somewhat unforeseen side effect, but without this "lowest match idx to run middleware for" concept we end up running deeper than maybe we expect on granular .data requests.
Say you have routes a and b and a has a clientLoader calling serverLoader, we'll end up with 2 reqeusts:
The navigational request will be GET /a/b.data?_routes=root,b and will calls both A and B middlewares ✅
But the targeted serverLoader call will be GET /a/b.data?_routes=a which is basically a direct call to the A loader endpoint, but will now call the middleware down through B, which feels like it could catch folks off guard?
| "child 1 loader start", | ||
| "child 2 loader start", | ||
| "child 2 loader end", | ||
| "child 1 loader end", |
There was a problem hiding this comment.
We no longer trim off routes for which we won't be calling loaders, so even though we won't call the "child" loaders here because we are rendering the child error boundary, we'll now call the child middlewares.
| handler: () => T extends true | ||
| ? MaybePromise<Response> | ||
| : MaybePromise<Record<string, DataStrategyResult>>, | ||
| errorHandler: (error: MiddlewareError) => unknown |
There was a problem hiding this comment.
I stopped passing a mutable keyedResults through to the handler and now you just return one from your handler directly and we propagate it back out from runMiddleware - this should make our user-facing runMiddleware function we plan to pass to dataStrategy easier
|
🤖 Hello there, We just published version Thanks! |
This reverts commit fb014ce.
|
🤖 Hello there, We just published version Thanks! |
…d-route-typegen * upstream/dev: (55 commits) Fix root loader data on initial load redirects in SPA mode (remix-run#13222) Ensure ancestor pathless/index routes are loaded via manifest requests (remix-run#13203) Fix shoulRevalidate behavior in clientLoader-only routes (remix-run#13221) Stop leaking internal MiddlewareError implementation detail (remix-run#13180) Fix validation of split route modules for root route (remix-run#13238) Fix `RequestHandler` `loadContext` type when middleware is enabled (remix-run#13204) Change middleware return type from void to undefined (remix-run#13199) update docs home page add API docs Fix error message typo Fix Windows CI (remix-run#13215) Remove Vite server hooks in child compiler plugins (remix-run#13184) Support flexible ordering of Vite plugins that override SSR environment (remix-run#13183) chore: format chore: Update version for release (remix-run#13175) Exit prerelease mode chore: Update version for release (pre) (remix-run#13174) Fix JSDoc types for context (remix-run#13170) Remove middleware depth restrictions (remix-run#13172) minor language improvements in context/middleware decision doc ...
No description provided.