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

Implement priority overrides for injected routes and redirects #9439

Merged
merged 37 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
9f6a7e9
Implement priority overrides for injected routes and redirects
Fryuni Dec 15, 2023
a6671a8
Fix ordering for route specificity
Fryuni Dec 15, 2023
bf8a333
Don't mix rules on tests
Fryuni Dec 15, 2023
31885a9
Detailed collision detection
Fryuni Dec 15, 2023
06bcec2
Add changeset
Fryuni Dec 15, 2023
8a61bb7
Remove TODO
Fryuni Dec 15, 2023
a475688
Merge branch 'main' into feat/injected-route-priority
Fryuni Dec 15, 2023
c211704
Add comments to clarify default values
Fryuni Dec 18, 2023
0c678b2
Merge branch 'main' into feat/injected-route-priority
Fryuni Dec 20, 2023
7379e1d
Update terminology
Fryuni Dec 20, 2023
a064b4e
Revert unrelated changes
Fryuni Dec 21, 2023
cea5439
WIP
Fryuni Jan 13, 2024
9dd4609
Merge branch 'main' into feat/injected-route-priority
Fryuni Jan 13, 2024
addd2d9
Refactor
Fryuni Jan 13, 2024
e3cbbe9
Fix typo and typing
Fryuni Jan 14, 2024
d5e2142
Merge branch 'main' into feat/injected-route-priority
ematipico Jan 15, 2024
322bb4d
chore: default to legacy
ematipico Jan 15, 2024
28df92f
chore: use experimental flag instead of option
ematipico Jan 15, 2024
a368d60
fix: do not throw an error on collisions
ematipico Jan 15, 2024
dc2760a
chore: fix regression
ematipico Jan 15, 2024
7cfbdbd
chore: use `continue` instead of `return`
ematipico Jan 15, 2024
61fc289
chore: fix tests but one
ematipico Jan 15, 2024
eb855b9
chore: Update test
Fryuni Jan 15, 2024
a8686b8
chore: Change remaining new error to warning
Fryuni Jan 15, 2024
e319eb4
chore: Test collision warnings
Fryuni Jan 15, 2024
bd93047
docs: Update docs of new config
Fryuni Jan 16, 2024
de3a8cb
docs: Improve changesets
Fryuni Jan 16, 2024
fdc7c90
chore: rename experimental flag
ematipico Jan 16, 2024
5671f41
chore: update changeset and docs
ematipico Jan 16, 2024
24c9c74
Sarah editing pass
sarah11918 Jan 16, 2024
7fc2482
nit: Align Markdown table
Fryuni Jan 16, 2024
30241b6
defined definitions!
sarah11918 Jan 16, 2024
fb78e03
added logging info to docs for experimental flag
sarah11918 Jan 16, 2024
dcf8ca4
Yan final boss review
sarah11918 Jan 16, 2024
9882de1
chore: Update flag name in tests
Fryuni Jan 16, 2024
a34647e
chore: Update flag name in tests
Fryuni Jan 16, 2024
dd8c15f
Merge remote-tracking branch 'origin/main' into feat/injected-route-p…
ematipico Jan 17, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions .changeset/short-keys-bow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"astro": patch
---

Fix inconsistency between route priorities of file routes, redirects and injected routes
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved


Now all groups are prioritized following the same rules:

- Routes with more path segments will take precedence over less specific routes. E.g. `/blog/post/[pid].astro` takes precedence over `/blog/[...slug].astro`.
- Static routes without path parameters will take precedence over dynamic routes. E.g. `/posts/create.astro` takes precedence over all the other routes in the example.
- Dynamic routes using named parameters take precedence over rest parameters. E.g. `/posts/[page].astro` takes precedence over `/posts/[...slug].astro`.
- Pre-rendered dynamic routes take precedence over server dynamic routes.
- Endpoints take precedence over pages. E.g. `/posts/[pid].ts` takes precedence over `/posts/[pid].astro`.
- If none of the rules above decide the order, routes are sorted alphabetically based on the default locale of your Node installation.
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 34 additions & 0 deletions .changeset/smooth-cobras-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
'astro': minor
---

Adds an experimental flag `stableRoutePriority` to make all routes be prioritized following the same stable rules.
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved

```js
// astro.config.mjs
export default defineConfig({
experimental: {
globalRoutePriority: true,
},
})
```

Enabling this feature makes redirects and injected routes be prioritized along with discovered file-based project
routes, behaving the same as if all routes were defined as files in the project.
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved

For the following scenario in SSR mode:
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved

- File-based route: `/blog/post/[pid]`
- File-based route: `/[page]`
- Injected route: `/blog/[...slug]`
- Redirect: `/blog/tags/[tag]` -> `/[tag]`
- Redirect: `/posts` -> `/blog`

URLs are handled with the following routes:
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved

| Page | Current Behavior | Global Routing Priority Behavior |
|--------------------|----------------------------------|------------------------------------|
| `/blog/tags/astro` | Injected route `/blog/[...slug]` | Redirect to `/tags/[tag]` |
| `/blog/post/0` | Injected route `/blog/[...slug]` | File-based route `/blog/post/[pid] |
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved
| `/posts` | File-based route `/[page]` | Redirect to `/blog` |

sarah11918 marked this conversation as resolved.
Show resolved Hide resolved
37 changes: 37 additions & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1566,6 +1566,33 @@ export interface AstroUserConfig {
* ```
*/
contentCollectionCache?: boolean;

/**
* @docs
* @name experimental.globalRoutePriority
* @type {boolean}
* @default `false`
* @version 4.2.0
* @description
*
* Prioritize redirects and injected routes along with discovered file-based project routes, behaving the same as if all routes were defined as files in the project.
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved
*
* For the following scenario in SSR mode:
* - File-based route: `/blog/post/[pid]`
* - File-based route: `/[page]`
* - Injected route: `/blog/[...slug]`
* - Redirect: `/blog/tags/[tag]` -> `/[tag]`
* - Redirect: `/posts` -> `/blog`
*
* URLs are handled with the following routes:
*
* | Page | Current Behavior | Global Routing Priority Behavior |
* | ------------------ | -------------------------------- | ---------------------------------- |
* | `/blog/tags/astro` | Injected route `/blog/[...slug]` | Redirect to `/tags/[tag]` |
* | `/blog/post/0` | Injected route `/blog/[...slug]` | File-based route `/blog/post/[pid] |
* | `/posts` | File-based route `/[page]` | Redirect to `/blog` |
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved
*/
globalRoutePriority?: boolean;
};
}

Expand All @@ -1587,6 +1614,15 @@ export interface AstroUserConfig {
*/
export type InjectedScriptStage = 'before-hydration' | 'head-inline' | 'page' | 'page-ssr';

/**
* IDs for different priorities of injected routes and redirects:
Fryuni marked this conversation as resolved.
Show resolved Hide resolved
* - "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 overriden by any project route on conflict.
sarah11918 marked this conversation as resolved.
Show resolved Hide resolved
*/
export type RoutePriorityOverride = 'normal' | 'legacy';

export interface InjectedRoute {
pattern: string;
entrypoint: string;
Expand Down Expand Up @@ -2431,6 +2467,7 @@ type RedirectConfig =
| {
status: ValidRedirectStatus;
destination: string;
priority?: RoutePriorityOverride;
};

export interface RouteData {
Expand Down
5 changes: 5 additions & 0 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ const ASTRO_CONFIG_DEFAULTS = {
experimental: {
optimizeHoistedScript: false,
contentCollectionCache: false,
globalRoutePriority: false,
},
} satisfies AstroUserConfig & { server: { open: boolean } };

Expand Down Expand Up @@ -393,6 +394,10 @@ export const AstroConfigSchema = z.object({
.boolean()
.optional()
.default(ASTRO_CONFIG_DEFAULTS.experimental.contentCollectionCache),
globalRoutePriority: z
.boolean()
.optional()
.default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority),
})
.strict(
`Invalid or outdated experimental feature.\nCheck for incorrect spelling or outdated Astro version.\nSee https://docs.astro.build/en/reference/configuration-reference/#experimental-flags for a list of all current experiments.`
Expand Down