-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: correctly emit middleware file name #7427
Conversation
🦋 Changeset detectedLatest commit: 75957fd The changes in this PR will be included in the next version bump. 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 |
5f95f0b
to
f45f721
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple suggestions below to make it more Rollup-style. In the future, I think we can also look into using this.emitFile
to emit a new chunk instead, so we don't have to specify a rollup input and configure entryFileNames
Would that work? I would aim to emit an entry point, not a chunk. |
4039318
to
59f594d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice the new refactor looks great now.
Would that work? I would aim to emit an entry point, not a chunk.
It should be possible to be used as an entrypoint too as long as you call emitFile
with preserveSignatures: 'strict'
59f594d
to
6a5d029
Compare
6a5d029
to
75957fd
Compare
Changes
The emission of the middleware file during the build still needed to be corrected. The code was working as expected, but the
middleware.mjs
wasn't emitted as expected, so I had to refactor how theplugin-middleware.ts
works.Notable changes:
import * as middleware
. This wasn't ideal. When doing so,rollup
creates a new export, which could cause some issue in the future;middleware
wrapper is not present anymore, and because of that, I had to review the types inside the code;This change is needed for other adapters to pick up the emitted code in a predictable way.
Testing
Current tests should pass
Docs
N/A