-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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: adapter features, deprecate astro configs #7839
Conversation
🦋 Changeset detectedLatest commit: af251d9 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.
Thanks for making the changes! This looks great to me.
Should we also validate if the deprecated configs are used in runtime, somewhere around here?
astro/packages/astro/src/core/config/config.ts
Lines 65 to 77 in 73eb4df
let legacyConfigKey: string | undefined; | |
for (const key of Object.keys(userConfig)) { | |
if (LEGACY_ASTRO_CONFIG_KEYS.has(key)) { | |
legacyConfigKey = key; | |
break; | |
} | |
} | |
if (legacyConfigKey) { | |
throw new AstroError({ | |
...AstroErrorData.ConfigLegacyKey, | |
message: AstroErrorData.ConfigLegacyKey.message(legacyConfigKey), | |
}); | |
} |
Also, the docs team were also interested in the API discussion for this, so might be good to run by them if this new syntax is better. cc @withastro/maintainers-docs
Two reasons why we can't do that:
|
38d243f
to
baffd37
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.
Not core docs, but I found a few typos
@@ -1400,6 +1400,17 @@ export interface DataEntryType { | |||
|
|||
export type GetDataEntryInfoReturnType = { data: Record<string, unknown>; rawData?: string }; | |||
|
|||
export interface AstroAdapterFeatures { | |||
/** | |||
* Creates and edge function that will communiate with the Astro middleware |
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.
* Creates and edge function that will communiate with the Astro middleware | |
* Creates an edge function that will communicate with the Astro middleware |
@@ -74,7 +74,11 @@ function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): V | |||
exports.push(`export { renderers };`); | |||
|
|||
// The middleware should not be imported by the pages | |||
if (!opts.settings.config.build.excludeMiddleware) { | |||
if ( | |||
// TODO: remover in Astro 4.0 |
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.
typo?
// TODO: remover in Astro 4.0 | |
// TODO: remove in Astro 4.0 |
I mean we can to the warning deprecations, and validate |
Changes
This PR deprecates
build.split
andbuild.excludeMiddleware
.After an internal discussion, we realised that these two config values should not live inside Astro, because they are only usable only with an adapter that can understand it
Because of that, we introduced the concept of
adapterFeatures
. These features are toggles that signal Astro to change the output of the build, and then the adapter needs to do something with those.Testing
I updated the current test to use the new configuration. The tests should pass.
Docs
/cc @withastro/maintainers-docs for feedback!