-
-
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
feat: astro features #7815
feat: astro features #7815
Conversation
🦋 Changeset detectedLatest commit: 728d4e7 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 |
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.
6f0715f
to
48c1146
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 general validation logic looks great! Got some questions/comments about some things
48c1146
to
83d15dd
Compare
321d44d
to
71ad64b
Compare
packages/integrations/netlify/src/integration-edge-functions.ts
Outdated
Show resolved
Hide resolved
87b3074
to
81e5db2
Compare
Should the values be lower-case? When we have string flags like this elsewhere in the code I'm pretty sure they are lower-case. |
5fc1afc
to
4b3085e
Compare
@matthewp that's addressed now, thank you |
7049810
to
2a84125
Compare
a606b49
to
f520318
Compare
Changes
Introducing Astro features. Astro features are features that astro has out the box, and sometimes users can opt-in.
Few examples:
staticOutput
if a user uses a specific adapter, the adapter needs to be able to serve those static assets. An example is Cloudflare, which doesn't support it.assets
: not supported by those adapters that are incompatible with nodeThis PR adds a new API that adapters can use when registering one. It's called
supportedFeatures
, and it's a new way to tell Astro if and when an adapter is able to support certain features that are enabled via configuration.The adapter can also tell Astro the kind of support:
For now, this new API/object is not mandatory, but it will be in Astro 4.0.
The PR also updates the adapters that we maintain, and it adds the feature map to them.
About
assets
There's a small caveat for the assets. Astro assets is a feature that is already turned on by default, meaning that Astro sets already a default service. This means that adapters that don't support assets will always get an error, which is inconvenient. I came up with a solution where we set the default after the adapter validation, although I don't like the solution very much, and I wonder if there's a better way to handle a case like this.
The assets feature is just an example; we might have the same problem with future features like this. What do you suggest?
Testing
I added new unit test cases and updated the existing ones.
Docs
I will create the changelog in another PR