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

feat(next): astro:routes:resolved #12329

Merged
merged 29 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
621d2d2
feat: work on astro:routes:resolved
florian-lefebvre Oct 28, 2024
fbb04e9
feat: work on jsdoc
florian-lefebvre Oct 29, 2024
98e77c9
feat: support origin
florian-lefebvre Oct 29, 2024
96f560b
misc
florian-lefebvre Oct 29, 2024
65f5b60
feat: add test
florian-lefebvre Oct 29, 2024
75a84dc
feat: add changeset
florian-lefebvre Oct 29, 2024
3a3b083
Update packages/astro/src/types/public/integrations.ts
florian-lefebvre Nov 1, 2024
ec16abf
feat: rename property
florian-lefebvre Nov 1, 2024
ce0fc54
Merge branch 'next' into feat/routes-resolved
florian-lefebvre Nov 1, 2024
7e26fa4
feat: handle partial manifest updates in dev
florian-lefebvre Nov 1, 2024
319b405
Merge branch 'next' into feat/routes-resolved
florian-lefebvre Nov 13, 2024
db92a98
feat: include more properties and rename origin things
florian-lefebvre Nov 13, 2024
fc4e42c
feat: update origin
florian-lefebvre Nov 13, 2024
ebb12f9
feat: deprecate properties instead of whole object
florian-lefebvre Nov 13, 2024
b377e0f
Update packages/astro/src/types/public/internal.ts
florian-lefebvre Nov 13, 2024
33b1165
feat: update test
florian-lefebvre Nov 13, 2024
5576af9
feat: work on test
florian-lefebvre Nov 13, 2024
46be686
feat: new assets
florian-lefebvre Nov 14, 2024
c9dcd7c
fix: test
florian-lefebvre Nov 14, 2024
24600f0
fix: take dev routes into account
florian-lefebvre Nov 14, 2024
ea9abf4
Merge branch 'next' into feat/routes-resolved
florian-lefebvre Nov 14, 2024
93a69e2
Update packages/astro/src/vite-plugin-astro-server/plugin.ts
florian-lefebvre Nov 15, 2024
e837aff
feat: address review
florian-lefebvre Nov 15, 2024
7db88f9
Merge branch 'next' into feat/routes-resolved
florian-lefebvre Nov 18, 2024
12088d0
fix: check and calls
florian-lefebvre Nov 18, 2024
e1f53d5
feat: address reviews
florian-lefebvre Nov 19, 2024
539d40f
Apply suggestions from code review
florian-lefebvre Nov 19, 2024
846c3a7
fix: normalize paths
florian-lefebvre Nov 20, 2024
cd62268
Merge branch 'next' into feat/routes-resolved
florian-lefebvre Nov 20, 2024
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
43 changes: 43 additions & 0 deletions .changeset/giant-ravens-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
'astro': minor
---

Adds a new `astro:routes:resolved` hook to the Integration API, and deprecates `routes` passed to `astro:build:done`

When building an integration, you can now get access to routes inside the `astro:routes:resolved` hook:

```js
const integration = () => {
return {
name: 'my-integration',
hooks: {
'astro:routes:resolved': ({ routes }) => {
console.log(routes)
}
}
}
}
```

This hook runs before `astro:config:done`, and whenever a route changes in development.

`routes` from `astro:build:done` is now deprecated. We recommend you use the new hook instead:
ematipico marked this conversation as resolved.
Show resolved Hide resolved

```diff
const integration = () => {
+ let routes
return {
name: 'my-integration',
hooks: {
+ 'astro:routes:resolved': (params) => {
+ routes = params.routes
+ },
'astro:build:done': ({
- routes
}) => {
console.log(routes)
}
}
}
}
```
3 changes: 2 additions & 1 deletion packages/astro/src/actions/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ export default function astroIntegrationActionsRouteHandler({
name: VIRTUAL_MODULE_ID,
hooks: {
async 'astro:config:setup'(params) {
params.injectRoute({
settings.injectedRoutes.push({
pattern: '/_actions/[...path]',
entrypoint: 'astro/actions/runtime/route.js',
prerender: false,
origin: 'core',
});

params.addMiddleware({
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/assets/endpoint/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,6 @@ function getImageEndpointData(
pathname: settings.config.image.endpoint.route,
prerender: false,
fallbackRoutes: [],
origin: 'core',
};
}
1 change: 1 addition & 0 deletions packages/astro/src/container/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ export class experimental_AstroContainer {
type,
fallbackRoutes: [],
isIndex: false,
origin: 'user',
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
};
}

Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/core/routing/astro-designed-error-pages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export const DEFAULT_404_ROUTE: RouteData = {
route: '/404',
fallbackRoutes: [],
isIndex: false,
origin: 'core',
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
};

export const DEFAULT_500_ROUTE: RouteData = {
Expand All @@ -29,6 +30,7 @@ export const DEFAULT_500_ROUTE: RouteData = {
route: '/500',
fallbackRoutes: [],
isIndex: false,
origin: 'core',
};

export function ensure404Route(manifest: ManifestData) {
Expand Down
8 changes: 7 additions & 1 deletion packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { routeComparator } from '../priority.js';
import { getRouteGenerator } from './generator.js';
import { getPattern } from './pattern.js';
import { getRoutePrerenderOption } from './prerender.js';
import { runHookRoutesResolved } from '../../../integrations/hooks.js';
const require = createRequire(import.meta.url);

interface Item {
Expand Down Expand Up @@ -255,6 +256,7 @@ function createFileBasedRoutes(
prerender,
fallbackRoutes: [],
distURL: [],
origin: 'user',
});
}
}
Expand All @@ -280,7 +282,7 @@ function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): Rou
const routes: RouteData[] = [];

for (const injectedRoute of settings.injectedRoutes) {
const { pattern: name, entrypoint, prerender: prerenderInjected } = injectedRoute;
const { pattern: name, entrypoint, prerender: prerenderInjected, origin } = injectedRoute;
const { resolved, component } = resolveInjectedRoute(entrypoint.toString(), config.root, cwd);

const segments = removeLeadingForwardSlash(name)
Expand Down Expand Up @@ -320,6 +322,7 @@ function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): Rou
prerender: prerenderInjected ?? prerender,
fallbackRoutes: [],
distURL: [],
origin,
});
}

Expand Down Expand Up @@ -389,6 +392,7 @@ function createRedirectRoutes(
redirectRoute: routeMap.get(destination),
fallbackRoutes: [],
distURL: [],
origin: 'user',
});
}

Expand Down Expand Up @@ -727,6 +731,8 @@ export async function createRouteManifest(
}
}

await runHookRoutesResolved({ routes, settings, logger });

return {
routes,
};
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/routing/manifest/serialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,6 @@ export function deserializeRouteData(rawRouteData: SerializedRouteData): RouteDa
return deserializeRouteData(fallback);
}),
isIndex: rawRouteData.isIndex,
origin: rawRouteData.origin,
};
}
1 change: 1 addition & 0 deletions packages/astro/src/core/server-islands/endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function getServerIslandRouteData(config: ConfigFields) {
isIndex: false,
fallbackRoutes: [],
route: SERVER_ISLAND_ROUTE,
origin: 'core',
};
return route;
}
Expand Down
38 changes: 36 additions & 2 deletions packages/astro/src/integrations/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import type {
import type {
AstroIntegration,
AstroRenderer,
BaseIntegrationHooks,
HookParameters,
IntegrationResolvedRoute,
IntegrationRouteData,
RouteOptions,
} from '../types/public/integrations.js';
Expand All @@ -39,7 +41,7 @@ async function withTakingALongTimeMsg<T>({
logger,
}: {
name: string;
hookName: string;
hookName: keyof BaseIntegrationHooks;
hookResult: T | Promise<T>;
timeoutMs?: number;
logger: Logger;
Expand Down Expand Up @@ -204,7 +206,7 @@ export async function runHookConfigSetup({
);
injectRoute.entrypoint = injectRoute.entryPoint as string;
}
updatedSettings.injectedRoutes.push(injectRoute);
updatedSettings.injectedRoutes.push({ ...injectRoute, origin: 'integration' });
},
addWatchFile: (path) => {
updatedSettings.watchFiles.push(path instanceof URL ? fileURLToPath(path) : path);
Expand Down Expand Up @@ -648,6 +650,38 @@ export async function runHookRouteSetup({
}
}

export async function runHookRoutesResolved({
routes,
settings,
logger,
}: { routes: Array<RouteData>; settings: AstroSettings; logger: Logger }) {
for (const integration of settings.config.integrations) {
if (integration?.hooks?.['astro:routes:resolved']) {
const integrationLogger = getLogger(integration, logger);

await withTakingALongTimeMsg({
name: integration.name,
hookName: 'astro:routes:resolved',
hookResult: integration.hooks['astro:routes:resolved']({
routes: routes.map((route) => toIntegrationResolvedRoute(route)),
logger: integrationLogger,
}),
logger,
});
}
}
}

function toIntegrationResolvedRoute(route: RouteData): IntegrationResolvedRoute {
return {
prerendered: route.prerender,
entrypoint: route.component,
pattern: route.route,
params: route.params,
origin: route.origin,
};
}

function toIntegrationRouteData(route: RouteData): IntegrationRouteData {
return {
route: route.route,
Expand Down
6 changes: 2 additions & 4 deletions packages/astro/src/types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ import type { ContentEntryType, DataEntryType } from './public/content.js';
import type {
AstroAdapter,
AstroRenderer,
InjectedRoute,
InjectedScriptStage,
InjectedType,
ResolvedInjectedRoute,
} from './public/integrations.js';
import type { RouteData } from './public/internal.js';
import type { InternalInjectedRoute, RouteData, ResolvedInjectedRoute } from './public/internal.js';
import type { DevToolbarAppEntry } from './public/toolbar.js';

export type SerializedRouteData = Omit<
Expand All @@ -35,7 +33,7 @@ export interface AstroSettings {
config: AstroConfig;
adapter: AstroAdapter | undefined;
preferences: AstroPreferences;
injectedRoutes: InjectedRoute[];
injectedRoutes: InternalInjectedRoute[];
resolvedInjectedRoutes: ResolvedInjectedRoute[];
pageExtensions: string[];
contentEntryTypes: ContentEntryType[];
Expand Down
50 changes: 39 additions & 11 deletions packages/astro/src/types/public/integrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { getToolbarServerCommunicationHelpers } from '../../integrations/ho
import type { DeepPartial } from '../../type-utils.js';
import type { AstroConfig } from './config.js';
import type { RefreshContentOptions } from './content.js';
import type { RouteData } from './internal.js';
import type { InternalInjectedRoute, RouteData } from './internal.js';
import type { DevToolbarAppEntry } from './toolbar.js';

export interface RouteOptions {
Expand Down Expand Up @@ -138,15 +138,7 @@ export type AstroAdapterFeatureMap = {
*/
export type InjectedScriptStage = 'before-hydration' | 'head-inline' | 'page' | 'page-ssr';

export interface InjectedRoute {
pattern: string;
entrypoint: string | URL;
prerender?: boolean;
}

export interface ResolvedInjectedRoute extends InjectedRoute {
resolvedEntryPoint?: URL;
}
export type InjectedRoute = Omit<InternalInjectedRoute, 'origin'>;

export interface InjectedType {
filename: string;
Expand Down Expand Up @@ -225,13 +217,18 @@ export interface BaseIntegrationHooks {
'astro:build:done': (options: {
pages: { pathname: string }[];
dir: URL;
/** @deprecated Use `routes` from `astro:routes:resolved` instead */
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
routes: IntegrationRouteData[];
logger: AstroIntegrationLogger;
}) => void | Promise<void>;
'astro:route:setup': (options: {
route: RouteOptions;
logger: AstroIntegrationLogger;
}) => void | Promise<void>;
'astro:routes:resolved': (options: {
routes: Array<IntegrationResolvedRoute>;
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
logger: AstroIntegrationLogger;
}) => void | Promise<void>;
}

export interface AstroIntegration {
Expand All @@ -248,10 +245,41 @@ export interface AstroIntegration {
*/
export type IntegrationRouteData = Omit<
RouteData,
'isIndex' | 'fallbackRoutes' | 'redirectRoute'
'isIndex' | 'fallbackRoutes' | 'redirectRoute' | 'origin'
> & {
/**
* {@link RouteData.redirectRoute}
*/
redirectRoute?: IntegrationRouteData;
};

export interface IntegrationResolvedRoute {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
/**
* The current **pattern** of the route. For example:
* - `src/pages/index.astro` has a pattern of `/`
* - `src/pages/blog/[...slug].astro` has a pattern of `/blog/[...slug]`
* - `src/pages/site/[blog]/[...slug].astro` has a pattern of `/site/[blog]/[...slug]`
*/
pattern: RouteData['route'];

/**
* Source component URL
*/
entrypoint: RouteData['component'];
Copy link
Member

Choose a reason for hiding this comment

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

Why do we change the internal naming? Wouldn't it create fragmentation? Also, we shifted into a pattern where "entrypoint" is URL or string, where the string is an absolute file path. In this case, these are string, and they are relative paths. I think we should keep component, so they are not misunderstood like the other "entrypoints" we have in other APIs.

Copy link
Member

Choose a reason for hiding this comment

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

I proposed changing the "component" name for the new public API because the value here is not necessarily a component. This is already true of RouteData['component'], the name is misleading.

I'm not sure whether entrypoint is the best name either. It can be:

  • A relative path when the route is a redirect
  • The absolute path to an Astro component when the route is a page
  • The import path (before resolving dependencies) of an Astro component when the route is a page
  • The absolute path to a JS file when the route is an API
  • The import path (before resolving dependencies) of a JS file when the route is an API

It is quite an overloaded field, so I think it needs a more generic name than "component".

Some options and my thoughts about them:

  • source: would be misleading for redirect since it is where it redirects to and not from
  • definition: I don't think it really fits with redirect
  • implementation: also doesn't fit with redirect
  • target: seems weird to say an Astro component is the target of a page but could work
  • reference: in the sense of a "reference implementation" for the page or API and the reference link for the redirect

Copy link
Member

Choose a reason for hiding this comment

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

What about origin or creator?

Copy link
Member Author

Choose a reason for hiding this comment

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

cc @Fryuni

Copy link
Member

Choose a reason for hiding this comment

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

Similarly, they feel weird for redirects because it is where you'd redirect to not from. Both would imply from to me.

I don't have any particularly strong feelings against any of the names Ema and I proposed, nor with component. I don't think this is worth blocking progress if it comes to it. It is just that none of them matches all 3 cases that can exist for this field. component is just worse than the alternatives for me since it directly implies one of the options when there are another 2.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah we can agree there isn't a perfect name, so we need to be good at documenting :)

I also don't have particularly strong feelings about a particular name


/**
* Whether the route is prerendered or not
*/
prerendered: RouteData['prerender'];
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved

/**
* Dynamic and spread route params
* ex. "/pages/[lang]/[...slug].astro" will output the params ['lang', '...slug']
*/
params: RouteData['params'];
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved

/**
* Whether the route comes from Astro core, an integration or the user's project
*/
origin: RouteData['origin'];
}
16 changes: 16 additions & 0 deletions packages/astro/src/types/public/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ export interface RouteData {
* - src/pages/blog/index.astro
*/
isIndex: boolean;

/**
* Whether the route comes from Astro core, an integration or the user's project
*/
origin: 'core' | 'integration' | 'user';
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down Expand Up @@ -284,3 +289,14 @@ export interface SSRMetadata {
}

export type SSRError = Error & ViteErrorPayload['err'];

export interface InternalInjectedRoute {
florian-lefebvre marked this conversation as resolved.
Show resolved Hide resolved
pattern: string;
entrypoint: string | URL;
prerender?: boolean;
origin: RouteData['origin'];
}

export interface ResolvedInjectedRoute extends InternalInjectedRoute {
resolvedEntryPoint?: URL;
}
Loading
Loading