-
-
Notifications
You must be signed in to change notification settings - Fork 52
Conversation
🦋 Changeset detectedLatest commit: dd0ada5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 packages
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 |
Does this require changes in |
Good call. Yes, it will. I will make a PR |
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.
Looks good! I was just wondering whether a word like "Replaces" or "combines" might be a be a more user-friendly word re: guiding them towards the changes they'll have to make (e.g. a find/replace)
Co-authored-by: Bjorn Lu <[email protected]>
1ae809f
to
16d6883
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.
The change works for me 👍 We can also do type casting since that's easier if you want, but runtime checks also looks good. I noticed the code already had type casting like here:
adapters/packages/vercel/src/image/shared.ts
Lines 7 to 8 in 16d6883
// Cast is necessary here because Vercel's types are slightly different from ours regarding allowed protocols. Behavior should be the same, however. | |
remotePatterns: (astroImageConfig.remotePatterns as VercelImageConfig['remotePatterns']) ?? [], |
Co-authored-by: Bjorn Lu <[email protected]>
Co-authored-by: Sarah Rainsberger <[email protected]>
Co-authored-by: Bjorn Lu <[email protected]>
Changes
Closes PLT-2550
This PR refactors the code of the Vercel adapter, and it now exports one single entry point for both static and serverless adapter.
The new code is inside
index.ts
. The code that differs between the new adapters is conditionally applied usingif (buidOutput === "server")
all over.Also, I took the opportunity to make the creation of the
config.json
a bit more readable with a classicif/else
, all those big ternaries were a bit hard to handle.Testing
The current tests should pass.
I only updated one single test, where Astro was throwing an error for an incorrect configuration.
Docs
withastro/docs#9755