From b5584fcfc8a0639dc3e02c94fa6bd82db7b1a89c Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Tue, 20 Aug 2024 17:07:03 -0400 Subject: [PATCH 1/4] Remove legacy route prioritization --- packages/astro/src/core/config/schema.ts | 5 - .../astro/src/core/routing/manifest/create.ts | 53 +++----- packages/astro/src/types/public/config.ts | 33 +---- .../astro/src/types/public/integrations.ts | 9 -- .../astro/test/units/routing/manifest.test.js | 126 ------------------ 5 files changed, 17 insertions(+), 209 deletions(-) diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 05b1cc3cc42f..0a149de53e39 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -87,7 +87,6 @@ export const ASTRO_CONFIG_DEFAULTS = { directRenderScript: false, contentCollectionCache: false, clientPrerender: false, - globalRoutePriority: false, serverIslands: false, contentIntellisense: false, env: { @@ -518,10 +517,6 @@ export const AstroConfigSchema = z.object({ .boolean() .optional() .default(ASTRO_CONFIG_DEFAULTS.experimental.clientPrerender), - globalRoutePriority: z - .boolean() - .optional() - .default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority), env: z .object({ schema: EnvSchema.optional(), diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 14980f63e5ad..e45c926af60e 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -9,7 +9,6 @@ import { bold } from 'kleur/colors'; import { toRoutingStrategy } from '../../../i18n/utils.js'; import { getPrerenderDefault } from '../../../prerender/utils.js'; import type { AstroConfig } from '../../../types/public/config.js'; -import type { RoutePriorityOverride } from '../../../types/public/integrations.js'; import type { RouteData, RoutePart } from '../../../types/public/internal.js'; import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js'; import { MissingIndexForInternationalization } from '../../errors/errors-data.js'; @@ -271,18 +270,11 @@ function createFileBasedRoutes( return routes; } -type PrioritizedRoutesData = Record; - -function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): PrioritizedRoutesData { +function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): RouteData[] { const { config } = settings; const prerender = getPrerenderDefault(config); - const routes: PrioritizedRoutesData = { - normal: [], - legacy: [], - }; - - const priority = computeRoutePriority(config); + const routes: RouteData[] = []; for (const injectedRoute of settings.injectedRoutes) { const { pattern: name, entrypoint, prerender: prerenderInjected } = injectedRoute; @@ -317,7 +309,7 @@ function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): Pri .map((p) => p.content); const route = joinSegments(segments); - routes[priority].push({ + routes.push({ type, // For backwards compatibility, an injected route is never considered an index route. isIndex: false, @@ -343,16 +335,12 @@ function createRedirectRoutes( { settings }: CreateRouteManifestParams, routeMap: Map, logger: Logger, -): PrioritizedRoutesData { +): RouteData[] { const { config } = settings; const trailingSlash = config.trailingSlash; - const routes: PrioritizedRoutesData = { - normal: [], - legacy: [], - }; + const routes: RouteData[] = []; - const priority = computeRoutePriority(settings.config); for (const [from, to] of Object.entries(settings.config.redirects)) { const segments = removeLeadingForwardSlash(from) .split(path.posix.sep) @@ -387,7 +375,7 @@ function createRedirectRoutes( ); } - routes[priority].push({ + routes.push({ type: 'redirect', // For backwards compatibility, a redirect is never considered an index route. isIndex: false, @@ -505,28 +493,26 @@ export function createRouteManifest( } const injectedRoutes = createInjectedRoutes(params); - for (const [, routes] of Object.entries(injectedRoutes)) { - for (const route of routes) { - routeMap.set(route.route, route); - } + for (const route of injectedRoutes) { + routeMap.set(route.route, route); } const redirectRoutes = createRedirectRoutes(params, routeMap, logger); const routes: RouteData[] = [ - ...injectedRoutes['legacy'].sort(routeComparator), - ...[...fileBasedRoutes, ...injectedRoutes['normal'], ...redirectRoutes['normal']].sort( + ...[ + ...fileBasedRoutes, + ...injectedRoutes, + ...redirectRoutes + ].sort( routeComparator, ), - ...redirectRoutes['legacy'].sort(routeComparator), ]; // Report route collisions - if (config.experimental.globalRoutePriority) { - for (const [index, higherRoute] of routes.entries()) { - for (const lowerRoute of routes.slice(index + 1)) { - detectRouteCollision(higherRoute, lowerRoute, config, logger); - } + for (const [index, higherRoute] of routes.entries()) { + for (const lowerRoute of routes.slice(index + 1)) { + detectRouteCollision(higherRoute, lowerRoute, config, logger); } } @@ -726,13 +712,6 @@ export function createRouteManifest( }; } -function computeRoutePriority(config: AstroConfig): RoutePriorityOverride { - if (config.experimental.globalRoutePriority) { - return 'normal'; - } - return 'legacy'; -} - function joinSegments(segments: RoutePart[][]): string { const arr = segments.map((segment) => { return segment.map((rp) => (rp.dynamic ? `[${rp.content}]` : rp.content)).join(''); diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts index 5dfe40f39a88..2decd2c84410 100644 --- a/packages/astro/src/types/public/config.ts +++ b/packages/astro/src/types/public/config.ts @@ -11,7 +11,7 @@ import type { AssetsPrefix } from '../../core/app/types.js'; import type { AstroConfigType } from '../../core/config/schema.js'; import type { Logger, LoggerLevel } from '../../core/logger/core.js'; import type { EnvSchema } from '../../env/schema.js'; -import type { AstroIntegration, RoutePriorityOverride } from './integrations.js'; +import type { AstroIntegration } from './integrations.js'; export type Locales = (string | { codes: string[]; path: string })[]; @@ -29,7 +29,6 @@ export type RedirectConfig = | { status: ValidRedirectStatus; destination: string; - priority?: RoutePriorityOverride; }; export type ServerConfig = { @@ -1582,36 +1581,6 @@ export interface AstroUserConfig { */ clientPrerender?: boolean; - /** - * @docs - * @name experimental.globalRoutePriority - * @type {boolean} - * @default `false` - * @version 4.2.0 - * @description - * - * Prioritizes redirects and injected routes equally alongside file-based project routes, following the same [route priority order rules](https://docs.astro.build/en/guides/routing/#route-priority-order) for all routes. - * - * This allows more control over routing in your project by not automatically prioritizing certain types of routes, and standardizes the route priority ordering for all routes. - * - * The following example shows which route will build certain page URLs when file-based routes, injected routes, and redirects are combined as shown below: - * - File-based route: `/blog/post/[pid]` - * - File-based route: `/[page]` - * - Injected route: `/blog/[...slug]` - * - Redirect: `/blog/tags/[tag]` -> `/[tag]` - * - Redirect: `/posts` -> `/blog` - * - * With `experimental.globalRoutingPriority` enabled (instead of Astro 4.0 default route priority order): - * - * - `/blog/tags/astro` is built by the redirect to `/tags/[tag]` (instead of the injected route `/blog/[...slug]`) - * - `/blog/post/0` is built by the file-based route `/blog/post/[pid]` (instead of the injected route `/blog/[...slug]`) - * - `/posts` is built by the redirect to `/blog` (instead of the file-based route `/[page]`) - * - * - * In the event of route collisions, where two routes of equal route priority attempt to build the same URL, Astro will log a warning identifying the conflicting routes. - */ - globalRoutePriority?: boolean; - /** * @docs * @name experimental.env diff --git a/packages/astro/src/types/public/integrations.ts b/packages/astro/src/types/public/integrations.ts index 129d11215881..ea463c961538 100644 --- a/packages/astro/src/types/public/integrations.ts +++ b/packages/astro/src/types/public/integrations.ts @@ -136,15 +136,6 @@ export interface AstroInternationalizationFeature { */ export type InjectedScriptStage = 'before-hydration' | 'head-inline' | 'page' | 'page-ssr'; -/** - * IDs for different priorities of injected routes and redirects: - * - "normal": Merge with discovered file-based project routes, behaving the same as if the route - * was defined as a file in the project. - * - "legacy": Use the old ordering of routes. Inject routes will override any file-based project route, - * and redirects will be overridden by any project route on conflict. - */ -export type RoutePriorityOverride = 'normal' | 'legacy'; - export interface InjectedRoute { pattern: string; entrypoint: string; diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js index 523d86e6adcd..518a2fc8f0e3 100644 --- a/packages/astro/test/units/routing/manifest.test.js +++ b/packages/astro/test/units/routing/manifest.test.js @@ -74,9 +74,6 @@ describe('routing - createRouteManifest', () => { root: fileURLToPath(root), base: '/search', trailingSlash: 'never', - experimental: { - globalRoutePriority: true, - }, }); settings.injectedRoutes = [ @@ -131,9 +128,6 @@ describe('routing - createRouteManifest', () => { root: fileURLToPath(root), base: '/search', trailingSlash: 'never', - experimental: { - globalRoutePriority: true, - }, }); const manifest = createRouteManifest({ @@ -171,9 +165,6 @@ describe('routing - createRouteManifest', () => { root: fileURLToPath(root), base: '/search', trailingSlash: 'never', - experimental: { - globalRoutePriority: true, - }, }); const manifest = createRouteManifest({ @@ -220,9 +211,6 @@ describe('routing - createRouteManifest', () => { root: fileURLToPath(root), base: '/search', trailingSlash: 'never', - experimental: { - globalRoutePriority: true, - }, }); const manifest = createRouteManifest({ @@ -262,58 +250,6 @@ describe('routing - createRouteManifest', () => { ]); }); - it('injected routes are sorted in legacy mode above filesystem routes', async () => { - const fs = createFs( - { - '/src/pages/index.astro': `

test

`, - '/src/pages/blog/[...slug].astro': `

test

`, - }, - root, - ); - const settings = await createBasicSettings({ - root: fileURLToPath(root), - output: 'server', - base: '/search', - trailingSlash: 'never', - }); - - settings.injectedRoutes = [ - { - pattern: '/contributing', - entrypoint: '@lib/legacy/static.astro', - }, - { - pattern: '/[...slug]', - entrypoint: '@lib/legacy/dynamic.astro', - }, - ]; - - const manifest = createRouteManifest({ - cwd: fileURLToPath(root), - settings, - fsMod: fs, - }); - - assert.deepEqual(getManifestRoutes(manifest), [ - { - route: '/contributing', - type: 'page', - }, - { - route: '/[...slug]', - type: 'page', - }, - { - route: '/blog/[...slug]', - type: 'page', - }, - { - route: '/', - type: 'page', - }, - ]); - }); - it('injected routes are sorted alongside filesystem routes', async () => { const fs = createFs( { @@ -327,9 +263,6 @@ describe('routing - createRouteManifest', () => { output: 'server', base: '/search', trailingSlash: 'never', - experimental: { - globalRoutePriority: true, - }, }); settings.injectedRoutes = [ @@ -370,53 +303,6 @@ describe('routing - createRouteManifest', () => { ]); }); - it('redirects are sorted in legacy mode below the filesystem routes', async () => { - const fs = createFs( - { - '/src/pages/index.astro': `

test

`, - '/src/pages/blog/contributing.astro': `

test

`, - }, - root, - ); - const settings = await createBasicSettings({ - root: fileURLToPath(root), - output: 'server', - base: '/search', - trailingSlash: 'never', - redirects: { - '/blog/[...slug]': '/', - '/blog/about': { - status: 302, - destination: '/another', - }, - }, - }); - const manifest = createRouteManifest({ - cwd: fileURLToPath(root), - settings, - fsMod: fs, - }); - - assert.deepEqual(getManifestRoutes(manifest), [ - { - route: '/blog/contributing', - type: 'page', - }, - { - route: '/', - type: 'page', - }, - { - route: '/blog/about', - type: 'redirect', - }, - { - route: '/blog/[...slug]', - type: 'redirect', - }, - ]); - }); - it('redirects are sorted alongside the filesystem routes', async () => { const fs = createFs( { @@ -440,9 +326,6 @@ describe('routing - createRouteManifest', () => { destination: '/another', }, }, - experimental: { - globalRoutePriority: true, - }, }); const manifest = createRouteManifest({ cwd: fileURLToPath(root), @@ -483,9 +366,6 @@ describe('routing - createRouteManifest', () => { base: '/search', trailingSlash: 'never', integrations: [], - experimental: { - globalRoutePriority: true, - }, }); settings.injectedRoutes = [ @@ -536,9 +416,6 @@ describe('routing - createRouteManifest', () => { base: '/search', trailingSlash: 'never', integrations: [], - experimental: { - globalRoutePriority: true, - }, }); const manifestOptions = { @@ -585,9 +462,6 @@ describe('routing - createRouteManifest', () => { redirects: { '/posts/a-[b].233': '/blog/a-[b].233', }, - experimental: { - globalRoutePriority: true, - }, }); settings.injectedRoutes = [ From 964e0ee5d3e4c8106d715992c90e520004564704 Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Wed, 21 Aug 2024 08:23:41 -0400 Subject: [PATCH 2/4] oops --- packages/astro/src/core/config/schema.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 28b18c93ef51..b1bf8799e15b 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -532,7 +532,7 @@ export const AstroConfigSchema = z.object({ validateSecrets: z .boolean() .optional() - .default(ASTRO_CONFIG_DEFAULTS.experimental.env.validateSecrets), + .default(ASTRO_CONFIG_DEFAULTS.env.validateSecrets), }) .strict() .optional(), From 4a78e745183805e46f69c2d1ec9e2e15953a7b5c Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Wed, 21 Aug 2024 13:28:39 -0400 Subject: [PATCH 3/4] Add a changeset --- .changeset/blue-boats-relax.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/blue-boats-relax.md diff --git a/.changeset/blue-boats-relax.md b/.changeset/blue-boats-relax.md new file mode 100644 index 000000000000..93d9b51ca220 --- /dev/null +++ b/.changeset/blue-boats-relax.md @@ -0,0 +1,9 @@ +--- +'astro': major +--- + +Unflag globalRoutePriority + +The previously experimental feature `globalRoutePriority` is now the default in Astro 5. + +This was a refactoring of route prioritization in Astro, making it so that injected routes, file-based routes, and redirects are all prioritized using the same logic. This feature has been enabled for all Starlight projects since it was added and should not affect most users. From 2938241d547cdd491787660b9a9525ce7f21c59b Mon Sep 17 00:00:00 2001 From: Matthew Phillips Date: Fri, 23 Aug 2024 08:26:38 -0500 Subject: [PATCH 4/4] Remove bad merge stuff --- packages/astro/src/core/config/schema.ts | 10 -- packages/astro/src/types/public/config.ts | 142 ---------------------- 2 files changed, 152 deletions(-) diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index b1bf8799e15b..abf1be876b80 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -526,16 +526,6 @@ export const AstroConfigSchema = z.object({ .boolean() .optional() .default(ASTRO_CONFIG_DEFAULTS.experimental.clientPrerender), - env: z - .object({ - schema: EnvSchema.optional(), - validateSecrets: z - .boolean() - .optional() - .default(ASTRO_CONFIG_DEFAULTS.env.validateSecrets), - }) - .strict() - .optional(), serverIslands: z .boolean() .optional() diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts index 4c2129e7c927..78f95869d735 100644 --- a/packages/astro/src/types/public/config.ts +++ b/packages/astro/src/types/public/config.ts @@ -1648,148 +1648,6 @@ export interface AstroUserConfig { */ clientPrerender?: boolean; - /** - * @docs - * @name experimental.env - * @type {object} - * @default `undefined` - * @version 4.10.0 - * @description - * - * Enables experimental `astro:env` features. - * - * The `astro:env` API lets you configure a type-safe schema for your environment variables, and indicate whether they should be available on the server or the client. Import and use your defined variables from the appropriate `/client` or `/server` module: - * - * ```astro - * --- - * import { API_URL } from "astro:env/client" - * import { API_SECRET_TOKEN } from "astro:env/server" - * - * const data = await fetch(`${API_URL}/users`, { - * method: "GET", - * headers: { - * "Content-Type": "application/json", - * "Authorization": `Bearer ${API_SECRET_TOKEN}` - * }, - * }) - * --- - * - * - * ``` - * - * To define the data type and properties of your environment variables, declare a schema in your Astro config in `experimental.env.schema`. The `envField` helper allows you define your variable as a string, number, or boolean and pass properties in an object: - * - * ```js - * // astro.config.mjs - * import { defineConfig, envField } from "astro/config" - * - * export default defineConfig({ - * experimental: { - * env: { - * schema: { - * API_URL: envField.string({ context: "client", access: "public", optional: true }), - * PORT: envField.number({ context: "server", access: "public", default: 4321 }), - * API_SECRET: envField.string({ context: "server", access: "secret" }), - * } - * } - * } - * }) - * ``` - * - * There are currently four data types supported: strings, numbers, booleans and enums. - * - * There are three kinds of environment variables, determined by the combination of `context` (client or server) and `access` (secret or public) settings defined in your [`env.schema`](#experimentalenvschema): - * - * - **Public client variables**: These variables end up in both your final client and server bundles, and can be accessed from both client and server through the `astro:env/client` module: - * - * ```js - * import { API_URL } from "astro:env/client" - * ``` - * - * - **Public server variables**: These variables end up in your final server bundle and can be accessed on the server through the `astro:env/server` module: - * - * ```js - * import { PORT } from "astro:env/server" - * ``` - * - * - **Secret server variables**: These variables are not part of your final bundle and can be accessed on the server through the `astro:env/server` module. The `getSecret()` helper function can be used to retrieve secrets not specified in the schema. Its implementation is provided by your adapter and defaults to `process.env`: - * - * ```js - * import { API_SECRET, getSecret } from "astro:env/server" - * - * const SECRET_NOT_IN_SCHEMA = getSecret("SECRET_NOT_IN_SCHEMA") // string | undefined - * ``` - * - * **Note:** Secret client variables are not supported because there is no safe way to send this data to the client. Therefore, it is not possible to configure both `context: "client"` and `access: "secret"` in your schema. - * - * For a complete overview, and to give feedback on this experimental API, see the [Astro Env RFC](https://github.com/withastro/roadmap/blob/feat/astro-env-rfc/proposals/0046-astro-env.md). - */ - env?: { - /** - * @docs - * @name experimental.env.schema - * @kind h4 - * @type {EnvSchema} - * @default `undefined` - * @version 4.10.0 - * @description - * - * An object that uses `envField` to define the data type (`string`, `number`, or `boolean`) and properties of your environment variables: `context` (client or server), `access` (public or secret), a `default` value to use, and whether or not this environment variable is `optional` (defaults to `false`). - * ```js - * // astro.config.mjs - * import { defineConfig, envField } from "astro/config" - * - * export default defineConfig({ - * experimental: { - * env: { - * schema: { - * API_URL: envField.string({ context: "client", access: "public", optional: true }), - * PORT: envField.number({ context: "server", access: "public", default: 4321 }), - * API_SECRET: envField.string({ context: "server", access: "secret" }), - * } - * } - * } - * }) - * ``` - */ - schema?: EnvSchema; - - /** - * @docs - * @name experimental.env.validateSecrets - * @kind h4 - * @type {boolean} - * @default `false` - * @version 4.11.6 - * @description - * - * Whether or not to validate secrets on the server when starting the dev server or running a build. - * - * By default, only public variables are validated on the server when starting the dev server or a build, and private variables are validated at runtime only. If enabled, private variables will also be checked on start. This is useful in some continuous integration (CI) pipelines to make sure all your secrets are correctly set before deploying. - * - * ```js - * // astro.config.mjs - * import { defineConfig, envField } from "astro/config" - * - * export default defineConfig({ - * experimental: { - * env: { - * schema: { - * // ... - * }, - * validateSecrets: true - * } - * } - * }) - * ``` - */ - validateSecrets?: boolean; - }; - /** * @docs * @name experimental.serverIslands