refactor: support middleware in entrypoint #14616
Conversation
|
github-actions
bot
added
pkg: integration
ematipico
force-pushed
the
env-api/middleware
branch
from
5fc70f4 to
a867fca
Compare
| constructor(manifest: SSRManifest, streaming = true, logger: Logger) { | ||
| super(manifest, streaming, logger); | ||
| this.logger = logger; | ||
| this.manifestData = routesList; |
There was a problem hiding this comment.
This isn't needed because it's done in base.ts
| export function getApp(dev = import.meta.env.DEV): BaseApp { | ||
| if (dev) { | ||
| const logger = createConsoleLogger('debug'); | ||
| return new DevApp(manifest, true, logger, { routes: routes.map((r) => r.routeData) }); |
|
I'm a little confused by the description here. This is about |
| contents.push(`const pageMap = new Map([\n ${pageMap.join(',\n ')}\n]);`); | ||
| exports.push(`export { pageMap }`); | ||
| const middleware = await this.resolve(MIDDLEWARE_MODULE_ID); | ||
| const middleware = await this.resolve(MIDDLEWARE_RESOLVED_MODULE_ID); |
There was a problem hiding this comment.
Why is this change needed?
There was a problem hiding this comment.
Because up until now the middleware virtual module was never imported via import. Now we do that, so we need a simple name, and the one with \0
There was a problem hiding this comment.
That's what I don't understand, why do we need to import the module with the null byte? Asking because my understanding is that Vite is supposed to be able to resolve the id without the null byte (which is kinda like an implementation detail of the virtual module)
Yes, and that's done by updating the |
|
@ematipico I don't see that from the code, it looks like this adds middleware to the manifest and that's what makes it work. Don't mean to be nit-picky, but |
|
@matthewp what do I need to address? My description? My code? If so, how? |
|
@ematipico yeah just the description, to specify that this fixes middleware support in the environment branch, and that the way that it fixes it is by adding them to them to the manifest. Separately, the tests are currently failing. |
| export async function loadMiddleware(moduleLoader: ModuleLoader) { | ||
| try { | ||
| return await moduleLoader.import(MIDDLEWARE_MODULE_ID); | ||
| return await moduleLoader.import(MIDDLEWARE_RESOLVED_MODULE_ID); |
There was a problem hiding this comment.
I think this should still import MIDDLEWARE_MODULE_ID to let Vite handle the module resolution. I'd be happy to be proved wrong tho
Changes
This PR fixes the middleware usage when calling the
getAppfunction in dev. The manifest didn't have themiddlewaregetter function, which is in charge of resolving the middleware module.This is now done by exposing the middleware virtual module in userland.
Testing
Added a new middleware and used some basic features:
Docs
N/A