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

Add private addPageExtension hook #3628

Merged
merged 2 commits into from
Jun 20, 2022
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
1 change: 1 addition & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,7 @@ export interface AstroConfig extends z.output<typeof AstroConfigSchema> {
// that is different from the user-exposed configuration.
// TODO: Create an AstroConfig class to manage this, long-term.
_ctx: {
pageExtensions: string[];
injectedRoutes: InjectedRoute[];
adapter: AstroAdapter | undefined;
renderers: AstroRenderer[];
Expand Down
72 changes: 40 additions & 32 deletions packages/astro/src/core/build/static-build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,46 +57,54 @@ export async function staticBuild(opts: StaticBuildOptions) {
const [renderers, mod] = pageData.preload;
const metadata = mod.$$metadata;

// Track client:only usage so we can map their CSS back to the Page they are used in.
const clientOnlys = Array.from(metadata.clientOnlyComponentPaths());
Copy link
Contributor

Choose a reason for hiding this comment

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

There's going to be merge conflicts after #3625 is merged. However all of this code goes away, so I think your changes won't be needed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, noticed this! Was glad to see this going away.

trackClientOnlyPageDatas(internals, pageData, clientOnlys);

const topLevelImports = new Set([
// Any component that gets hydrated
// 'components/Counter.jsx'
// { 'components/Counter.jsx': 'counter.hash.js' }
...metadata.hydratedComponentPaths(),
// Client-only components
...clientOnlys,
// The client path for each renderer
...renderers
.filter((renderer) => !!renderer.clientEntrypoint)
.map((renderer) => renderer.clientEntrypoint!),
]);

// Add hoisted scripts
const hoistedScripts = new Set(metadata.hoistedScriptPaths());
if (hoistedScripts.size) {
const uniqueHoistedId = JSON.stringify(Array.from(hoistedScripts).sort());
let moduleId: string;

// If we're already tracking this set of hoisted scripts, get the unique id
if (uniqueHoistedIds.has(uniqueHoistedId)) {
moduleId = uniqueHoistedIds.get(uniqueHoistedId)!;
} else {
// Otherwise, create a unique id for this set of hoisted scripts
moduleId = `/astro/hoisted.js?q=${uniqueHoistedIds.size}`;
uniqueHoistedIds.set(uniqueHoistedId, moduleId);
if (metadata) {
Copy link
Member Author

Choose a reason for hiding this comment

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

All this noise is just moving anything that accesses metadata inside an if (metadata) block. With custom pages, metadata might not be defined.

// Any component that gets hydrated
// 'components/Counter.jsx'
// { 'components/Counter.jsx': 'counter.hash.js' }
for (const hydratedComponentPath of metadata.hydratedComponentPaths()) {
topLevelImports.add(hydratedComponentPath);
}

// Track client:only usage so we can map their CSS back to the Page they are used in.
const clientOnlys = Array.from(metadata.clientOnlyComponentPaths());
trackClientOnlyPageDatas(internals, pageData, clientOnlys);

// Client-only components
for (const clientOnly of clientOnlys) {
topLevelImports.add(clientOnly)
}
topLevelImports.add(moduleId);

// Make sure to track that this page uses this set of hoisted scripts
if (internals.hoistedScriptIdToPagesMap.has(moduleId)) {
const pages = internals.hoistedScriptIdToPagesMap.get(moduleId);
pages!.add(astroModuleId);
} else {
internals.hoistedScriptIdToPagesMap.set(moduleId, new Set([astroModuleId]));
internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedScripts);

// Add hoisted scripts
const hoistedScripts = new Set(metadata.hoistedScriptPaths());
if (hoistedScripts.size) {
const uniqueHoistedId = JSON.stringify(Array.from(hoistedScripts).sort());
let moduleId: string;

// If we're already tracking this set of hoisted scripts, get the unique id
if (uniqueHoistedIds.has(uniqueHoistedId)) {
moduleId = uniqueHoistedIds.get(uniqueHoistedId)!;
} else {
// Otherwise, create a unique id for this set of hoisted scripts
moduleId = `/astro/hoisted.js?q=${uniqueHoistedIds.size}`;
uniqueHoistedIds.set(uniqueHoistedId, moduleId);
}
topLevelImports.add(moduleId);

// Make sure to track that this page uses this set of hoisted scripts
if (internals.hoistedScriptIdToPagesMap.has(moduleId)) {
const pages = internals.hoistedScriptIdToPagesMap.get(moduleId);
pages!.add(astroModuleId);
} else {
internals.hoistedScriptIdToPagesMap.set(moduleId, new Set([astroModuleId]));
internals.hoistedScriptIdToHoistedMap.set(moduleId, hoistedScripts);
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ export async function validateConfig(
// First-Pass Validation
const result = {
...(await AstroConfigRelativeSchema.parseAsync(userConfig)),
_ctx: { scripts: [], renderers: [], injectedRoutes: [], adapter: undefined },
_ctx: { pageExtensions: [], scripts: [], renderers: [], injectedRoutes: [], adapter: undefined },
};
// Final-Pass Validation (perform checks that require the full config object)
if (
Expand Down
16 changes: 12 additions & 4 deletions packages/astro/src/core/render/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import type {
} from '../../@types/astro';
import type { LogOptions } from '../logger/core.js';

import { renderHead, renderPage } from '../../runtime/server/index.js';
import { renderHead, renderPage, renderComponent } from '../../runtime/server/index.js';
import { getParams } from '../routing/params.js';
import { createResult } from './result.js';
import { callGetStaticPaths, findPathItemByKey, RouteCache } from './route-cache.js';
Expand Down Expand Up @@ -126,8 +126,6 @@ export async function render(
const Component = await mod.default;
if (!Component)
throw new Error(`Expected an exported Astro component but received typeof ${typeof Component}`);
if (!Component.isAstroComponentFactory)
throw new Error(`Unable to SSR non-Astro component (${route?.component})`);

const result = createResult({
links,
Expand All @@ -146,7 +144,17 @@ export async function render(
ssr,
});

let page = await renderPage(result, Component, pageProps, null);
let page: Awaited<ReturnType<typeof renderPage>>;
if (!Component.isAstroComponentFactory) {
const props: Record<string, any> = { ...(pageProps ?? {}), 'server:root': true };
natemoo-re marked this conversation as resolved.
Show resolved Hide resolved
const html = await renderComponent(result, Component.name, Component, props, null);
page = {
type: 'html',
html: html.toString()
}
Comment on lines +148 to +154
Copy link
Member Author

Choose a reason for hiding this comment

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

If the module exports a non-astro file, we pass it through our component pipeline instead, which will use any provided renderers to render the component

} else {
page = await renderPage(result, Component, pageProps, null);
}

if (page.type === 'response') {
return page;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export function createRouteManifest(
): ManifestData {
const components: string[] = [];
const routes: RouteData[] = [];
const validPageExtensions: Set<string> = new Set(['.astro', '.md']);
const validPageExtensions: Set<string> = new Set(['.astro', '.md', ...config._ctx.pageExtensions]);
const validEndpointExtensions: Set<string> = new Set(['.js', '.ts']);

function walk(dir: string, parentSegments: RoutePart[][], parentParams: string[]) {
Expand Down
18 changes: 15 additions & 3 deletions packages/astro/src/integrations/index.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
import type { AddressInfo } from 'net';
import type { ViteDevServer } from 'vite';
import { AstroConfig, AstroRenderer, BuildConfig, RouteData } from '../@types/astro.js';
import { AstroConfig, AstroIntegration, AstroRenderer, BuildConfig, RouteData } from '../@types/astro.js';
import ssgAdapter from '../adapter-ssg/index.js';
import type { SerializedSSRManifest } from '../core/app/types';
import type { PageBuildData } from '../core/build/types';
import { mergeConfig } from '../core/config.js';
import type { ViteConfigWithSSR } from '../core/create-vite.js';
import { isBuildingToSSR } from '../core/util.js';

type Hooks<Hook extends keyof AstroIntegration['hooks'], Fn = AstroIntegration['hooks'][Hook]> = Fn extends (...args: any) => any ? Parameters<Fn>[0] : never;

export async function runHookConfigSetup({
config: _config,
command,
Expand All @@ -34,7 +36,7 @@ export async function runHookConfigSetup({
* ```
*/
if (integration?.hooks?.['astro:config:setup']) {
await integration.hooks['astro:config:setup']({
const hooks: Hooks<'astro:config:setup'> = {
config: updatedConfig,
command,
addRenderer(renderer: AstroRenderer) {
Expand All @@ -49,7 +51,17 @@ export async function runHookConfigSetup({
injectRoute: (injectRoute) => {
updatedConfig._ctx.injectedRoutes.push(injectRoute);
},
});
}
// Semi-private `addPageExtension` hook
Object.defineProperty(hooks, 'addPageExtension', {
value: (...input: (string|string[])[]) => {
const exts = (input.flat(Infinity) as string[]).map(ext => `.${ext.replace(/^\./, '')}`);
updatedConfig._ctx.pageExtensions.push(...exts);
},
writable: false,
enumerable: false
})
Comment on lines +55 to +63
Copy link
Member Author

Choose a reason for hiding this comment

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

Not that private, but it works! It is intentionally not enumerable or exposed in the types, only there if you're looking for it.

await integration.hooks['astro:config:setup'](hooks);
}
}
return updatedConfig;
Expand Down
7 changes: 7 additions & 0 deletions packages/astro/src/runtime/server/hydration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { serializeListValue } from './util.js';
const HydrationDirectives = ['load', 'idle', 'media', 'visible', 'only'];

interface ExtractedProps {
isPage: boolean;
hydration: {
directive: string;
value: string;
Expand All @@ -24,10 +25,16 @@ interface ExtractedProps {
// Finds these special props and removes them from what gets passed into the component.
export function extractDirectives(inputProps: Record<string | number, any>): ExtractedProps {
let extracted: ExtractedProps = {
isPage: false,
hydration: null,
props: {},
};
for (const [key, value] of Object.entries(inputProps)) {
if (key.startsWith('server:')) {
if (key === 'server:root') {
extracted.isPage = true;
}
}
if (key.startsWith('client:')) {
if (!extracted.hydration) {
extracted.hydration = {
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export async function renderComponent(
const { renderers } = result._metadata;
const metadata: AstroComponentMetadata = { displayName };

const { hydration, props } = extractDirectives(_props);
const { hydration, isPage, props } = extractDirectives(_props);
let html = '';
let needsHydrationScript = hydration && determineIfNeedsHydrationScript(result);
let needsDirectiveScript =
Expand Down Expand Up @@ -317,6 +317,9 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
}

if (!hydration) {
if (isPage) {
return html;
}
Comment on lines +320 to +322
Copy link
Member Author

Choose a reason for hiding this comment

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

For hydration to work, we need to avoid stripping out astro-fragment on the top-level page, so we inject a server:root prop to track that.

return markHTMLString(html.replace(/\<\/?astro-fragment\>/g, ''));
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { defineConfig } from 'rollup'
import test from './integration.js'

export default defineConfig({
integrations: [test()]
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export default function() {
return {
name: '@astrojs/test-integration',
hooks: {
'astro:config:setup': ({ addPageExtension }) => {
addPageExtension('.mjs')
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@test/integration-add-page-extension",
"type": "module",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<h1>Hello world!</h1>
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// Convulted test case, rexport astro file from new `.mjs` page
import Test from '../components/test.astro';
export default Test;
19 changes: 19 additions & 0 deletions packages/astro/test/integration-add-page-extension.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';

describe('Integration addPageExtension', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({ root: './fixtures/integration-add-page-extension/' });
await fixture.build();
});

it('supports .mjs files', async () => {
const html = await fixture.readFile('/test/index.html');
const $ = cheerio.load(html);
expect($('h1').text()).to.equal('Hello world!');
});
});
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.