feat: connect vercel edge middleware to astro#7523
Merged
ematipico merged 6 commits intofeat/vercel-edge-middlewarefrom Jun 30, 2023
Merged
feat: connect vercel edge middleware to astro#7523ematipico merged 6 commits intofeat/vercel-edge-middlewarefrom
ematipico merged 6 commits intofeat/vercel-edge-middlewarefrom
Conversation
🦋 Changeset detectedLatest commit: 06a5c8a 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 |
matthewp
reviewed
Jun 29, 2023
| * prototype is the root `Object.prototype`. This includes objects created | ||
| * using object literals, but not for instance for class instances. | ||
| */ | ||
| function isPlainObject(value: unknown): value is object { |
Contributor
There was a problem hiding this comment.
We have packages/internal-helpers now. I could see some of the things like this going there. Not blocking though, just an idea.
Member
Author
There was a problem hiding this comment.
I thought about it, and wasn't sure what to do. I will move the function there!
matthewp
approved these changes
Jun 29, 2023
bluwy
reviewed
Jun 29, 2023
Member
That's because after the SSR build step, So somewhere is accidentally importing esbuild in the compiled runtime code maybe. I'm not really sure and it's hard to debug these stuff. |
Member
Author
|
!preview vercel-edge-middleware |
Contributor
|
Member
Author
|
I tested the feature and the middleware works! I could see my logs, and I could see the locals working and rendered in the Astro page, I could see the new Here's the plan: I will merge this PR once the CI passes. Then I will create a final PR that will merge all the new code to |
bluwy
reviewed
Jun 30, 2023
| analytics?: boolean; | ||
| imageService?: boolean; | ||
| imagesConfig?: VercelImageConfig; | ||
| createLocals?: CreateLocals; |
Member
There was a problem hiding this comment.
Do we want to remove this option since we're using the file now?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
This PR connects the Vercel Edge Middleware to the Astro middleware:
trySerializeLocalsto serialise thelocals. This is important because theselocalswill travel via headers, and we need to keep them when they are transferred from the vercel middleware to the Astro middleware;created a new API inside theAs per @bluwy suggestion, the new API is actually a new key file called@astrojs/verceladapter, this API is calledcreateLocalsand allows users to initialize theirlocalswith anything they want. The function accepts an object that containsrequestandcontext. You can check Vercel documentation;vercel-edge-middleware, it receives an object withrequestandcontextandesbuildwill bundle the file.localstoapp.render. Theselocalsare extracted from the headers of the request;Testing
I plan to create a preview release, create a new project and deploy it on Vercel.
Also, I added a snapshot test for the generated middleware. The reason is that it's very difficult to see how's the code generated, especially for reviewers. Please let me know how do feel about it; I am happy to remove it.
Docs
N/A Later when I know the APIs are stable and settled among the team members.