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

Use new compiler resolvePath option #5133

Merged
merged 11 commits into from
Oct 21, 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
5 changes: 5 additions & 0 deletions .changeset/dry-dragons-greet.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix `.css?raw` usage
5 changes: 5 additions & 0 deletions .changeset/gentle-insects-run.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Update `@astrojs/compiler` and use the new `resolvePath` option. This allows removing much of the runtime code, which should improve rendering performance for Astro and MDX pages.
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,4 @@
import Fake from '../abc.astro';
---
<h1>Testing unresolved frontmatter import</h1>
<Fake />
2 changes: 1 addition & 1 deletion packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
"test:e2e:match": "playwright test -g"
},
"dependencies": {
"@astrojs/compiler": "^0.27.1",
"@astrojs/compiler": "^0.28.0",
"@astrojs/language-server": "^0.26.2",
"@astrojs/markdown-remark": "^1.1.3",
"@astrojs/telemetry": "^1.0.1",
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type { SerializedSSRManifest } from '../core/app/types';
import type { PageBuildData } from '../core/build/types';
import type { AstroConfigSchema } from '../core/config';
import type { AstroCookies } from '../core/cookies';
import type { AstroComponentFactory, Metadata } from '../runtime/server';
import type { AstroComponentFactory } from '../runtime/server';
export type {
MarkdownHeading,
MarkdownMetadata,
Expand Down Expand Up @@ -968,7 +968,6 @@ export type AsyncRendererComponentFn<U> = (

/** Generic interface for a component (Astro, Svelte, React, etc.) */
export interface ComponentInstance {
$$metadata: Metadata;
default: AstroComponentFactory;
css?: string[];
getStaticPaths?: (options: GetStaticPathsOptions) => GetStaticPathsResult;
Expand Down
32 changes: 0 additions & 32 deletions packages/astro/src/core/build/vite-plugin-css.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,38 +223,6 @@ export function rollupPluginAstroBuildCSS(options: PluginOptions): VitePlugin[]
});
output.source = minifiedCSS;
}
} else if (output.type === 'chunk') {
// vite:css-post removes "pure CSS" JavaScript chunks, that is chunks that only contain a comment
// about it being a CSS module. We need to keep these chunks around because Astro
// re-imports all modules as their namespace `import * as module1 from 'some/path';
// in order to determine if one of them is a side-effectual web component.
// If we ever get rid of that feature, the code below can be removed.
for (const [imp, bindings] of Object.entries(output.importedBindings)) {
if (imp.startsWith('chunks/') && !bundle[imp] && output.code.includes(imp)) {
// This just creates an empty chunk module so that the main entry module
// that is importing it doesn't break.
const depChunk: OutputChunk = {
type: 'chunk',
fileName: imp,
name: imp,
facadeModuleId: imp,
code: `/* Pure CSS chunk ${imp} */ ${bindings
.map((b) => `export const ${b} = {};`)
.join('')}`,
dynamicImports: [],
implicitlyLoadedBefore: [],
importedBindings: {},
imports: [],
referencedFiles: [],
exports: Array.from(bindings),
isDynamicEntry: false,
isEntry: false,
isImplicitEntry: false,
modules: {},
};
bundle[imp] = depChunk;
}
}
}
}
}
Expand Down
42 changes: 6 additions & 36 deletions packages/astro/src/core/compile/compile.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import type { TransformResult } from '@astrojs/compiler';
import path from 'path';
import type { AstroConfig } from '../../@types/astro';
import type { TransformStyle } from './types';

import { transform } from '@astrojs/compiler';
import { AstroErrorCodes } from '../errors.js';
import { prependForwardSlash, removeLeadingForwardSlashWindows } from '../path.js';
import { AggregateError, resolveJsToTs, viteID } from '../util.js';
import { prependForwardSlash } from '../path.js';
import { AggregateError, resolvePath, viteID } from '../util.js';
import { createStylePreprocessor } from './style.js';

type CompilationCache = Map<string, CompileResult>;
Expand Down Expand Up @@ -37,13 +36,7 @@ async function compile({
// 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, {
// For Windows compat, prepend filename with `/`.
// Note this is required because the compiler uses URLs to parse and built paths.
// TODO: Ideally the compiler could expose a `resolvePath` function so that we can
// unify how we handle path in a bundler-agnostic way.
// At the meantime workaround with a slash and remove them in `astro:postprocess`
// when they are used in `client:component-path`.
pathname: prependForwardSlash(filename),
pathname: filename,
projectRoot: config.root.toString(),
site: config.site?.toString(),
sourcefile: filename,
Expand All @@ -54,6 +47,9 @@ async function compile({
// TODO: baseline flag
experimentalStaticExtraction: true,
preprocessStyle: createStylePreprocessor(transformStyle, cssDeps, cssTransformErrors),
async resolvePath(specifier) {
return resolvePath(specifier, filename);
},
})
.catch((err) => {
// throw compiler errors here if encountered
Expand Down Expand Up @@ -88,32 +84,6 @@ async function compile({
},
});

// Also fix path before returning. Example original resolvedPaths:
// - @astrojs/preact/client.js
// - @/components/Foo.vue
// - /Users/macos/project/src/Foo.vue
// - /C:/Windows/project/src/Foo.vue
for (const c of compileResult.clientOnlyComponents) {
c.resolvedPath = removeLeadingForwardSlashWindows(c.resolvedPath);
// The compiler trims .jsx by default, prevent this
if (c.specifier.endsWith('.jsx') && !c.resolvedPath.endsWith('.jsx')) {
c.resolvedPath += '.jsx';
}
if (path.isAbsolute(c.resolvedPath)) {
c.resolvedPath = resolveJsToTs(c.resolvedPath);
}
}
for (const c of compileResult.hydratedComponents) {
c.resolvedPath = removeLeadingForwardSlashWindows(c.resolvedPath);
// The compiler trims .jsx by default, prevent this
if (c.specifier.endsWith('.jsx') && !c.resolvedPath.endsWith('.jsx')) {
c.resolvedPath += '.jsx';
}
if (path.isAbsolute(c.resolvedPath)) {
c.resolvedPath = resolveJsToTs(c.resolvedPath);
}
}

return compileResult;
}

Expand Down
11 changes: 0 additions & 11 deletions packages/astro/src/core/render/dev/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { isPage, resolveIdToUrl } from '../../util.js';
import { createRenderContext, renderPage as coreRenderPage } from '../index.js';
import { filterFoundRenderers, loadRenderer } from '../renderer.js';
import { RouteCache } from '../route-cache.js';
import { collectMdMetadata } from '../util.js';
import { getStylesForURL } from './css.js';
import type { DevelopmentEnvironment } from './environment';
import { getScriptsForURL } from './scripts.js';
Expand Down Expand Up @@ -94,16 +93,6 @@ export async function preload({
const renderers = await loadRenderers(env.viteServer, env.settings);
// Load the module from the Vite SSR Runtime.
const mod = (await env.viteServer.ssrLoadModule(fileURLToPath(filePath))) as ComponentInstance;
if (env.viteServer.config.mode === 'development' || !mod?.$$metadata) {
return [renderers, mod];
}

// append all nested markdown metadata to mod.$$metadata
const modGraph = await env.viteServer.moduleGraph.getModuleByUrl(fileURLToPath(filePath));
if (modGraph) {
await collectMdMetadata(mod.$$metadata, modGraph, env.viteServer);
}

return [renderers, mod];
}

Expand Down
62 changes: 0 additions & 62 deletions packages/astro/src/core/render/util.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,3 @@
import type { ModuleNode, ViteDevServer } from 'vite';
import type { Metadata } from '../../runtime/server/metadata.js';

/** Check if a URL is already valid */
export function isValidURL(url: string): boolean {
try {
new URL(url);
return true;
} catch (e) {}
return false;
}

// https://vitejs.dev/guide/features.html#css-pre-processors
export const STYLE_EXTENSIONS = new Set([
'.css',
Expand All @@ -22,59 +10,9 @@ export const STYLE_EXTENSIONS = new Set([
'.less',
]);

// duplicate const from vite-plugin-markdown
// can't import directly due to Deno bundling issue
// (node fs import failing during prod builds)
const MARKDOWN_IMPORT_FLAG = '?mdImport';

const cssRe = new RegExp(
`\\.(${Array.from(STYLE_EXTENSIONS)
.map((s) => s.slice(1))
.join('|')})($|\\?)`
);
export const isCSSRequest = (request: string): boolean => cssRe.test(request);

// During prod builds, some modules have dependencies we should preload by hand
// Ex. markdown files imported asynchronously or via Astro.glob(...)
// This calls each md file's $$loadMetadata to discover those dependencies
// and writes all results to the input `metadata` object
const seenMdMetadata = new Set<string>();
export async function collectMdMetadata(
metadata: Metadata,
modGraph: ModuleNode,
viteServer: ViteDevServer
) {
const importedModules = [...(modGraph?.importedModules ?? [])];
await Promise.all(
importedModules.map(async (importedModule) => {
// recursively check for importedModules
if (!importedModule.id || seenMdMetadata.has(importedModule.id)) return;

seenMdMetadata.add(importedModule.id);
await collectMdMetadata(metadata, importedModule, viteServer);

if (!importedModule?.id?.endsWith(MARKDOWN_IMPORT_FLAG)) return;

const mdSSRMod = await viteServer.ssrLoadModule(importedModule.id);
const mdMetadata = (await mdSSRMod.$$loadMetadata?.()) as Metadata;
if (!mdMetadata) return;

for (let mdMod of mdMetadata.modules) {
mdMod.specifier = mdMetadata.resolvePath(mdMod.specifier);
metadata.modules.push(mdMod);
}
for (let mdHoisted of mdMetadata.hoisted) {
metadata.hoisted.push(mdHoisted);
}
for (let mdHydrated of mdMetadata.hydratedComponents) {
metadata.hydratedComponents.push(mdHydrated);
}
for (let mdClientOnly of mdMetadata.clientOnlyComponents) {
metadata.clientOnlyComponents.push(mdClientOnly);
}
for (let mdHydrationDirective of mdMetadata.hydrationDirectives) {
metadata.hydrationDirectives.add(mdHydrationDirective);
}
})
);
}
14 changes: 13 additions & 1 deletion packages/astro/src/core/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import path from 'path';
import resolve from 'resolve';
import slash from 'slash';
import { fileURLToPath, pathToFileURL } from 'url';
import type { ErrorPayload, ViteDevServer } from 'vite';
import { ErrorPayload, normalizePath, ViteDevServer } from 'vite';
import type { AstroConfig, AstroSettings, RouteType } from '../@types/astro';
import { prependForwardSlash, removeTrailingForwardSlash } from './path.js';

Expand Down Expand Up @@ -227,6 +227,18 @@ export function resolveJsToTs(filePath: string) {
return filePath;
}

/**
* Resolve the hydration paths so that it can be imported in the client
*/
export function resolvePath(specifier: string, importer: string) {
if (specifier.startsWith('.')) {
const absoluteSpecifier = path.resolve(path.dirname(importer), specifier);
return resolveJsToTs(normalizePath(absoluteSpecifier));
} else {
return specifier;
}
}

export const AggregateError =
typeof (globalThis as any).AggregateError !== 'undefined'
? (globalThis as any).AggregateError
Expand Down
20 changes: 3 additions & 17 deletions packages/astro/src/jsx/babel.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import type { PluginObj } from '@babel/core';
import * as t from '@babel/types';
import npath from 'path';
import { normalizePath } from 'vite';
import { resolveJsToTs } from '../core/util.js';
import { resolvePath } from '../core/util.js';
import { HydrationDirectiveProps } from '../runtime/server/hydration.js';
import type { PluginMetadata } from '../vite-plugin-astro/types';

Expand Down Expand Up @@ -217,13 +215,7 @@ export default function astroJSX(): PluginObj {

const meta = path.getData('import');
if (meta) {
let resolvedPath: string;
if (meta.path.startsWith('.')) {
resolvedPath = normalizePath(npath.resolve(npath.dirname(state.filename!), meta.path));
resolvedPath = resolveJsToTs(resolvedPath);
} else {
resolvedPath = meta.path;
}
const resolvedPath = resolvePath(meta.path, state.filename!);

if (isClientOnly) {
(state.file.metadata as PluginMetadata).astro.clientOnlyComponents.push({
Expand Down Expand Up @@ -297,13 +289,7 @@ export default function astroJSX(): PluginObj {
}
}
}
let resolvedPath: string;
if (meta.path.startsWith('.')) {
resolvedPath = normalizePath(npath.resolve(npath.dirname(state.filename!), meta.path));
resolvedPath = resolveJsToTs(resolvedPath);
} else {
resolvedPath = meta.path;
}
const resolvedPath = resolvePath(meta.path, state.filename!);
if (isClientOnly) {
(state.file.metadata as PluginMetadata).astro.clientOnlyComponents.push({
exportName: meta.name,
Expand Down
2 changes: 0 additions & 2 deletions packages/astro/src/runtime/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ export { createAstro } from './astro-global.js';
export { renderEndpoint } from './endpoint.js';
export { escapeHTML, HTMLBytes, HTMLString, markHTMLString, unescapeHTML } from './escape.js';
export { renderJSX } from './jsx.js';
export type { Metadata } from './metadata';
export { createMetadata } from './metadata.js';
export {
addAttribute,
defineScriptVars,
Expand Down
Loading