brisa-build / brisa

The Web Platform Framework.
https://brisa.build
MIT License
449 stars 12 forks source link

`req.renderInitiator` is always `INITIAL_REQUEST` on the middleware #584

Closed aralroca closed 1 month ago

aralroca commented 1 month ago

It should be 'INITIAL_REQUEST' | 'SPA_NAVIGATION' | 'SERVER_ACTION' to have this control from the middleware. It is likely that its value will be saved after the middleware call. It would be nice if this value was already set correctly by default.

aralroca commented 1 month ago

I'm wondering if rename this API to req.initiator to also add API endpoints here. It will be a getter, so until it is called the value will not be calculated, and it will only be calculated the first time, so it is not a performance cost.

What do you think @AlbertSabate ?

AlbertSabate commented 1 month ago

I agree with the change. renderInitiaor will sound confusing if you include api requests. Doing it like this on the middleware we could do something like:

// middleware.ts
import type { RequestContext } from 'brisa';

export default function middleware(req: RequestContext) {
  switch(req.initiator) {
    case 'INITIAL_REQUEST': ...
    case 'SPA_NAVIGATION': ...
    case 'SERVER_ACTION': ...
    case 'API_REQUEST': ...
    default: should never hit this one.
  }
}

Also, give more flexibility to add more cases in the future if necessary, not only tight to rendering.

Note: Changing RequestContext will also affect components. We could do a MiddlewareContext? I think for components should remain as it is, pretty clear. Also, API_Request would make no sense on the component.