Skip to content
This repository has been archived by the owner on Apr 6, 2023. It is now read-only.

fix(nuxt): process middleware after plugins #4645

Merged
merged 11 commits into from
May 2, 2022
Merged

Conversation

danielroe
Copy link
Member

@danielroe danielroe commented Apr 26, 2022

πŸ”— Linked issue

resolves nuxt/nuxt#13712, see also nuxt/nuxt#13679, resolves nuxt/nuxt#13884

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This PR changes middleware behaviour to defer processing middleware until after plugins are initialised. This means that middleware will have access to plugin injections, and that plugins have the chance to add middleware themselves.

In addition (and this is the breaking change), it skips processing middleware on client-side for server-side-rendered routes, aligning with Nuxt 2.

Potential for future improvement - .client middleware that does run on client-side but not on server

SSR:

  1. set up router + current route (by pushing it)
  2. add middleware
  3. process plugins
  4. replace current route with itself, triggering middleware

CSR:

  1. set up router + await ready
  2. add middleware
  3. process plugins
  4. replace current route with itself, triggering middleware

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@danielroe danielroe requested review from atinux and pi0 April 26, 2022 21:58
@danielroe danielroe self-assigned this Apr 26, 2022
@netlify
Copy link

netlify bot commented Apr 26, 2022

βœ… Deploy Preview for nuxt3-docs canceled.

Name Link
πŸ”¨ Latest commit b3af100
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt3-docs/deploys/626fa9e48d928c0009eb2ae5

@danielroe danielroe changed the title fix!: process middleware after plugins fix(nuxt)!: process middleware after plugins Apr 26, 2022
@danielroe danielroe added bug Something isn't working app πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage labels Apr 26, 2022
@atinux
Copy link
Member

atinux commented Apr 27, 2022

Beautiful work @danielroe ❀️

How can we add a test case to confirm the behaviour and avoid any potential regression in the future?

@atinux
Copy link
Member

atinux commented Apr 27, 2022

Happy to merge anyway and test internally 😊

@pi0
Copy link
Member

pi0 commented Apr 27, 2022

I'm wondering if we can keep client/server behavior consistent by either executing middleware on initial render or not. If server-only middleware on initial is desired, it can be easily guarded with if (process.server )

@danielroe danielroe changed the title fix(nuxt)!: process middleware after plugins fix(nuxt): process middleware after plugins Apr 29, 2022
@atinux
Copy link
Member

atinux commented Apr 29, 2022

I am not sure about this, I believe middleware shall only be run once. This is the behavior I have done in Nuxt 2 on purpose.

What is the advantage of rerunning them on client-side?

@pi0
Copy link
Member

pi0 commented Apr 29, 2022

I believe middleware shall only be run once.

They are. Like anything else during hydration and same as plugins, they run once on the server and once on the client side.

In general, I don't think there is any advantage that we are even calling middleware on initial rendering. It is a really strange use case that could be easily replicated with a nuxt plugin already and removed all this complexity for pushing route and ordering... But if we are doing this, it should be consistent IMO. We already introduced this change on Nuxt 3 and breaking it once again doesn't makes sense.

@byf77
Copy link

byf77 commented Apr 29, 2022

I think it has no sense you replicate inital middleware server side and client side.
In Nuxt 2 it was executing only on Server Side on page load.

Suppose you'll use a middleware global for fetch user on load page, the fetch will be executed on both side.. first request by server side and second one by client side.
It's a wasting of memory.

There is an issue about it too: nuxt/nuxt#13679

Maybe a solution is open a Discussion about it, and lets talk with community too.
@atinux @pi0

@danielroe
Copy link
Member Author

At the moment, this PR simply fixes a couple of bugs; it doesn't change the current behaviour, which runs middleware on client and server. It's not meant to make a decision on the question of whether to run middleware on initial client-side load. Let's have that conversation in nuxt/nuxt#13679; we can always follow up with a further PR reverting e14384e (#4645).

(We could also support .client and .server suffixes for middleware and it would be good to have only one breaking change rather than two in a row.)

@pi0 pi0 merged commit 4826918 into main May 2, 2022
@pi0 pi0 deleted the fix/middleware-plugin-ordering branch May 2, 2022 10:00
@vis97c
Copy link
Contributor

vis97c commented May 8, 2022

Why does a "global" middleware run only on the server? If people think they only need it on the server just use a process.server conditional. I have a usecase to run it twice but it seems like it is not possible with the current implementation.

@pi0 What are your thoughts on this?

@danielroe
Copy link
Member Author

Please see my immediately preceding comment above. This PR did not end up changing the fact that middleware runs on both client and server.

See nuxt/nuxt#13679 for an issue requesting this to change and run only once.

@danielroe danielroe added the 3.x label Jan 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
3.x app bug Something isn't working πŸ”¨ p3-minor-bug Priority 3: a bug in an edge case that only affects very specific usage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Route in plugin is always / Loading priority [Plugins & Middleware]
5 participants