-
-
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
refactor: rework supportedAstroFeatures #11806
Conversation
🦋 Changeset detectedLatest commit: d835c43 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 |
supportKind: AdapterSupport, | ||
adapterMessage?: string, | ||
) { | ||
switch (supportKind) { |
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.
Merged the different functions into one here that way you get an exhaustive check on the switch, felt nicer
export type AdapterSupportsKind = | ||
(typeof AdapterFeatureStability)[keyof typeof AdapterFeatureStability]; |
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.
Moved the source of truth from the type to the runtime check here, that way they're always accurate
} | ||
if (supportValue === AdapterFeatureStability.STABLE) { | ||
return true; | ||
} else if (hasCorrectConfig()) { |
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 previous check here was weird, it only checked if the config was correct for the unsupported value, but you need to always check it, otherwise you get warning for every experimental feature, even the ones you don't use?
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.
We have moved the adapter to https://github.com/withastro/adapters, so this PR can't be merged here anymore.
I would offer to port over the changes to the new repo for you, just let me know if you want me to do that.
@Princesseuh do you want me to port over the changes of the adapter related code, or are you going to do it yourself? |
ill do it myself, no worries |
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.
This PR is blocked because it contains a minor
changeset. A reviewer will merge this at the next release if approved.
will port the changes to the adapters repo
Changes
supportedAstroFeatures
on adapters is quite static, and the reality is often more nuanced. For instance, for Sharp, sometimes it's not supported with the default configuration, but with certain config it can be, but even then, it might only be on pre-rendered pages etc.This PR does a few things:
assets
section,"limited"
valueTesting
Updated tests, and the current ones should pass
Docs
Will do!