Skip to content
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

Remove legacy route prioritization #11798

Merged
merged 5 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/blue-boats-relax.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 0 additions & 5 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ export const ASTRO_CONFIG_DEFAULTS = {
directRenderScript: false,
contentCollectionCache: false,
clientPrerender: false,
globalRoutePriority: false,
serverIslands: false,
contentIntellisense: false,
contentLayer: false,
Expand Down Expand Up @@ -527,10 +526,6 @@ export const AstroConfigSchema = z.object({
.boolean()
.optional()
.default(ASTRO_CONFIG_DEFAULTS.experimental.clientPrerender),
globalRoutePriority: z
.boolean()
.optional()
.default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority),
serverIslands: z
.boolean()
.optional()
Expand Down
53 changes: 16 additions & 37 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -271,18 +270,11 @@ function createFileBasedRoutes(
return routes;
}

type PrioritizedRoutesData = Record<RoutePriorityOverride, RouteData[]>;

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;
Expand Down Expand Up @@ -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,
Expand All @@ -343,16 +335,12 @@ function createRedirectRoutes(
{ settings }: CreateRouteManifestParams,
routeMap: Map<string, RouteData>,
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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Comment on lines 512 to 517
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a note, this collision report is the warning part that is currently incorrect due to this prioritization happening before the export const prerender is read from the files.

The entire prioritization is affected, but this is the part that yells at the users, so it is more visible.


Expand Down Expand Up @@ -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('');
Expand Down
33 changes: 1 addition & 32 deletions packages/astro/src/types/public/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 })[];

Expand All @@ -29,7 +29,6 @@ export type RedirectConfig =
| {
status: ValidRedirectStatus;
destination: string;
priority?: RoutePriorityOverride;
};

export type ServerConfig = {
Expand Down Expand Up @@ -1649,36 +1648,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.serverIslands
Expand Down
9 changes: 0 additions & 9 deletions packages/astro/src/types/public/integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading