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.

Sign up for GitHub

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

Improve head injection behavior #2436

Merged
merged 5 commits into from
Mar 2, 2022
+38 −57
Merged

Improve head injection behavior #2436

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a766758
feat: add renderHead util to server
Jan 20, 2022
eee6fb7
feat: remove `layouts` from config, Vite plugin
Feb 25, 2022
b9adc1d
fix: improve head injection during rendering
Mar 1, 2022
6479566
chore: update compiler
Mar 2, 2022
b499ffd
fix: do not escape links
Mar 2, 2022
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
2 changes: 1 addition & 1 deletion packages/astro/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
"test:match": "mocha --timeout 20000 -g"
},
"dependencies": {
"@astrojs/compiler": "^0.12.0-next.5",
"@astrojs/compiler": "^0.12.0-next.8",
"@astrojs/language-server": "^0.8.6",
"@astrojs/markdown-remark": "^0.6.2",
"@astrojs/prism": "0.4.0",
Expand Down
11 changes: 0 additions & 11 deletions packages/astro/src/@types/astro.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,17 +303,6 @@ export type Params = Record<string, string | undefined>;

export type Props = Record<string, unknown>;

export interface RenderPageOptions {
request: {
params?: Params;
url: URL;
canonicalURL: URL;
};
children: any[];
props: Props;
css?: string[];
}

type Body = string;

export interface EndpointOutput<Output extends Body = Body> {
Expand Down
9 changes: 0 additions & 9 deletions packages/astro/src/core/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ export const AstroConfigSchema = z.object({
.optional()
.default('./src/pages')
.transform((val) => new URL(val)),
layouts: z
.string()
.optional()
.default('./src/layouts')
.transform((val) => new URL(val)),
public: z
.string()
.optional()
Expand Down Expand Up @@ -101,10 +96,6 @@ export async function validateConfig(userConfig: any, root: string): Promise<Ast
.string()
.default('./src/pages')
.transform((val) => new URL(addTrailingSlash(val), fileProtocolRoot)),
layouts: z
.string()
.default('./src/layouts')
.transform((val) => new URL(addTrailingSlash(val), fileProtocolRoot)),
public: z
.string()
.default('./public')
Expand Down
11 changes: 9 additions & 2 deletions packages/astro/src/core/render/core.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ComponentInstance, EndpointHandler, MarkdownRenderOptions, Params, Props, Renderer, RouteData, SSRElement } from '../../@types/astro';
import type { LogOptions } from '../logger.js';

import { renderEndpoint, renderPage } from '../../runtime/server/index.js';
import { renderEndpoint, renderHead, renderToString } from '../../runtime/server/index.js';
import { getParams } from '../routing/index.js';
import { createResult } from './result.js';
import { findPathItemByKey, RouteCache, callGetStaticPaths } from './route-cache.js';
Expand Down Expand Up @@ -97,7 +97,14 @@ export async function render(opts: RenderOptions): Promise<string> {
scripts,
});

let html = await renderPage(result, Component, pageProps, null);
let html = await renderToString(result, Component, pageProps, null);

// handle final head injection if it hasn't happened already
if (html.indexOf("<!--astro:head:injected-->") == -1) {
html = await renderHead(result) + html;
}
Comment on lines +102 to +105
Copy link
Member Author

Choose a reason for hiding this comment

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

Slight deviation from RFC0007... if we haven't injected any head elements yet, the user may not have rendered a <head> element so we just prepend them to the file.

// cleanup internal state flags
html = html.replace("<!--astro:head:injected-->", '');
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove our flag for a nicer output.


// inject <!doctype html> if missing (TODO: is a more robust check needed for comments, etc.?)
if (!legacyBuild && !/<!doctype html/i.test(html)) {
Expand Down
50 changes: 24 additions & 26 deletions packages/astro/src/runtime/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export class AstroComponent {
const html = htmlParts[i];
const expression = expressions[i];

yield _render(unescapeHTML(html));
yield unescapeHTML(html);
yield _render(expression);
}
}
Expand Down Expand Up @@ -405,20 +405,6 @@ export function defineScriptVars(vars: Record<any, any>) {
return output;
}

// Calls a component and renders it into a string of HTML
export async function renderToString(result: SSRResult, componentFactory: AstroComponentFactory, props: any, children: any) {
const Component = await componentFactory(result, props, children);
let template = await renderAstroComponent(Component);
return unescapeHTML(template);
}

// Filter out duplicate elements in our set
const uniqueElements = (item: any, index: number, all: any[]) => {
const props = JSON.stringify(item.props);
const children = item.children;
return index === all.findIndex((i) => JSON.stringify(i.props) === props && i.children == children);
};

// Renders an endpoint request to completion, returning the body.
export async function renderEndpoint(mod: EndpointHandler, params: any) {
const method = 'get';
Expand All @@ -433,15 +419,34 @@ export async function renderEndpoint(mod: EndpointHandler, params: any) {
return body;
}

// Calls a component and renders it into a string of HTML
export async function renderToString(result: SSRResult, componentFactory: AstroComponentFactory, props: any, children: any) {
const Component = await componentFactory(result, props, children);
let template = await renderAstroComponent(Component);

// <!--astro:head--> injected by compiler
// Must be handled at the end of the rendering process
if (template.indexOf('<!--astro:head-->') > -1) {
template = template.replace('<!--astro:head-->', await renderHead(result));
}
Comment on lines +429 to +431
Copy link
Member Author

Choose a reason for hiding this comment

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

<!--astro:head--> is injected by the compiler. If we see this, it's at the end of the <head> element and we should inject the links/styles/scripts.

return template;
}

// Filter out duplicate elements in our set
const uniqueElements = (item: any, index: number, all: any[]) => {
const props = JSON.stringify(item.props);
const children = item.children;
return index === all.findIndex((i) => JSON.stringify(i.props) === props && i.children == children);
};


// Renders a page to completion by first calling the factory callback, waiting for its result, and then appending
// styles and scripts into the head.
export async function renderPage(result: SSRResult, Component: AstroComponentFactory, props: any, children: any) {
const template = await renderToString(result, Component, props, children);
export async function renderHead(result: SSRResult) {
Comment on lines -438 to +445
Copy link
Member Author

Choose a reason for hiding this comment

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

No more renderPage entrypoint, just use renderToString.

const styles = Array.from(result.styles)
.filter(uniqueElements)
.map((style) => {
const styleChildren = !result._metadata.legacyBuild ? '' : style.children;

return renderElement('style', {
children: styleChildren,
props: { ...style.props, 'astro-style': true },
Expand All @@ -462,17 +467,10 @@ export async function renderPage(result: SSRResult, Component: AstroComponentFac
if (needsHydrationStyles) {
styles.push(renderElement('style', { props: { 'astro-style': true }, children: 'astro-root, astro-fragment { display: contents; }' }));
}

const links = Array.from(result.links)
.filter(uniqueElements)
.map((link) => renderElement('link', link, false));

// inject styles & scripts at end of <head>
let headPos = template.indexOf('</head>');
if (headPos === -1) {
return links.join('\n') + styles.join('\n') + scripts.join('\n') + template; // if no </head>, prepend styles & scripts
}
return template.substring(0, headPos) + links.join('\n') + styles.join('\n') + scripts.join('\n') + template.substring(headPos);
return unescapeHTML(links.join('\n') + styles.join('\n') + scripts.join('\n') + '\n' + '<!--astro:head:injected-->');
Copy link
Member Author

Choose a reason for hiding this comment

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

We also add a <!--astro:head:injected--> comment to flag that we've already injected the links/styles/scripts.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand after reading the code below.

}

export async function renderAstroComponent(component: InstanceType<typeof AstroComponent>) {
Expand Down
4 changes: 0 additions & 4 deletions packages/astro/src/vite-plugin-astro/compile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,8 @@ function safelyReplaceImportPlaceholder(code: string) {
const configCache = new WeakMap<AstroConfig, CompilationCache>();

async function compile(config: AstroConfig, filename: string, source: string, viteTransform: TransformHook, opts: { ssr: boolean }): Promise<CompileResult> {
// pages and layouts should be transformed as full documents (implicit <head> <body> etc)
// everything else is treated as a fragment
const filenameURL = new URL(`file://${filename}`);
const normalizedID = fileURLToPath(filenameURL);
const isPage = normalizedID.startsWith(fileURLToPath(config.pages)) || normalizedID.startsWith(fileURLToPath(config.layouts));
const pathname = filenameURL.pathname.substr(config.projectRoot.pathname.length - 1);

let rawCSSDeps = new Set<string>();
Expand All @@ -47,7 +44,6 @@ async function compile(config: AstroConfig, filename: string, source: string, vi
// use `sourcemap: "both"` so that sourcemap is included in the code
// result passed to esbuild, but also available in the catch handler.
const transformResult = await transform(source, {
as: isPage ? 'document' : 'fragment',
Copy link
Member Author

Choose a reason for hiding this comment

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

No more as: 'document'|'fragment'! We no longer care, every Astro file is parsed the same.

pathname,
projectRoot: config.projectRoot.toString(),
site: config.buildOptions.site,
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@
jsonpointer "^5.0.0"
leven "^3.1.0"

"@astrojs/compiler@^0.12.0-next.5":
version "0.12.0-next.5"
resolved "https://registry.yarnpkg.com/@astrojs/compiler/-/compiler-0.12.0-next.5.tgz#4e6d27c74787777522395018f2497dab4a032c77"
integrity sha512-4YVPRrB9JJhxoNC9PWN2zpGE7SXRAXcyCouawbd24iyBl4g9aRoQN12XA0qQZkbea9/NNLe9f2yhFMubM2CrJQ==
"@astrojs/compiler@^0.12.0-next.8":
version "0.12.0-next.8"
resolved "https://registry.yarnpkg.com/@astrojs/compiler/-/compiler-0.12.0-next.8.tgz#a15792791790aaeeaf944797635a40a56f806975"
integrity sha512-HeREaw5OR5J7zML+/LxhrqUr57571kyNXL4HD2pU929oevhx3PQ37PQ0FkD5N65X9YfO+gcoEO6whl76vtSZag==
dependencies:
typescript "^4.3.5"

Expand Down