From 272769d72394005d298470887eb2537d51c0bace Mon Sep 17 00:00:00 2001 From: Drew Powers <1369770+drwpow@users.noreply.github.com> Date: Tue, 15 Jun 2021 14:53:16 -0600 Subject: [PATCH] Improve asset resolution in Astro (#437) * Improve asset resolution in Astro Fixes #96 * Add docs, changeset * Fix collection resolution --- .changeset/loud-carrots-trade.md | 5 +++ docs/syntax.md | 33 +++++++++++++++++++ packages/astro/src/build/page.ts | 23 +++++-------- packages/astro/src/build/util.ts | 12 +++++-- packages/astro/src/runtime.ts | 5 ++- packages/astro/src/search.ts | 28 +++++++++------- packages/astro/test/astro-components.test.js | 1 - packages/astro/test/astro-resolve.test.js | 19 +++++++++++ .../fixtures/astro-resolve/public/svg.svg | 1 + .../astro-resolve/snowpack.config.json | 3 ++ .../astro-resolve/src/pages/index.astro | 12 +++++++ .../fixtures/astro-resolve/src/pages/svg.svg | 1 + 12 files changed, 111 insertions(+), 32 deletions(-) create mode 100644 .changeset/loud-carrots-trade.md create mode 100644 packages/astro/test/astro-resolve.test.js create mode 100644 packages/astro/test/fixtures/astro-resolve/public/svg.svg create mode 100644 packages/astro/test/fixtures/astro-resolve/snowpack.config.json create mode 100644 packages/astro/test/fixtures/astro-resolve/src/pages/index.astro create mode 100644 packages/astro/test/fixtures/astro-resolve/src/pages/svg.svg diff --git a/.changeset/loud-carrots-trade.md b/.changeset/loud-carrots-trade.md new file mode 100644 index 000000000..069e2a4e8 --- /dev/null +++ b/.changeset/loud-carrots-trade.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Improve asset resolution diff --git a/docs/syntax.md b/docs/syntax.md index 5fb940118..62b73104c 100644 --- a/docs/syntax.md +++ b/docs/syntax.md @@ -147,6 +147,39 @@ Inside of an expression, you must wrap multiple elements in a Fragment. Fragment | Special Characters | ` ` | `{'\xa0'}` or `{String.fromCharCode(160)}` | | Attributes | `dash-case` | `camelCase` | +### URL resolution + +It’s important to note that Astro **won’t** transform HTML references for you. For example, consider an `` tag with a relative `src` attribute inside `src/pages/about.astro`: + +```html + + +``` + +Since `src/pages/about.astro` will build to `/about/index.html`, you may not have expected that image to live at `/about/thumbnail.png`. So to fix this, choose either of two options: + +#### Option 1: Absolute URLs + +```html + + +``` + +The recommended approach is to place files within `public/*`. This references a file it `public/thumbnail.png`, which will resolve to `/thumbnail.png` at the final build (since `public/` ends up at `/`). + +#### Option 2: Asset import references + +```jsx +--- +// ✅ Correct: references src/thumbnail.png +import thumbnailSrc from './thumbnail.png'; +--- + + +``` + +If you’d prefer to organize assets alongside Astro components, you may import the file in JavaScript inside the component script. This works as intended but this makes `thumbnail.png` harder to reference in other parts of your app, as its final URL isn’t easily-predictable (unlike assets in `public/*`, where the final URL is guaranteed to never change). + ### TODO: Composition (Slots) [code-ext]: https://marketplace.visualstudio.com/items?itemName=astro-build.astro-vscode diff --git a/packages/astro/src/build/page.ts b/packages/astro/src/build/page.ts index df76f77ef..e19cf097b 100644 --- a/packages/astro/src/build/page.ts +++ b/packages/astro/src/build/page.ts @@ -91,18 +91,13 @@ export async function buildCollectionPage({ astroConfig, filepath, runtime, site export async function buildStaticPage({ astroConfig, buildState, filepath, runtime }: PageBuildOptions): Promise { const { pages: pagesRoot } = astroConfig; const url = filepath.pathname.replace(pagesRoot.pathname, '/').replace(/(index)?\.(astro|md)$/, ''); - - // build page in parallel - await Promise.all([ - runtime.load(url).then((result) => { - if (result.statusCode !== 200) throw new Error((result as any).error); - const outFile = path.posix.join(url, '/index.html'); - buildState[outFile] = { - srcPath: filepath, - contents: result.contents, - contentType: 'text/html', - encoding: 'utf8', - }; - }), - ]); + const result = await runtime.load(url); + if (result.statusCode !== 200) throw new Error((result as any).error); + const outFile = path.posix.join(url, '/index.html'); + buildState[outFile] = { + srcPath: filepath, + contents: result.contents, + contentType: 'text/html', + encoding: 'utf8', + }; } diff --git a/packages/astro/src/build/util.ts b/packages/astro/src/build/util.ts index 3132956e4..9dc78b453 100644 --- a/packages/astro/src/build/util.ts +++ b/packages/astro/src/build/util.ts @@ -21,13 +21,21 @@ export function getDistPath(specifier: string, { astroConfig, srcPath }: { astro const fileLoc = new URL(specifier, srcPath); const projectLoc = fileLoc.pathname.replace(projectRoot.pathname, ''); + const ext = path.extname(fileLoc.pathname); - const isPage = fileLoc.pathname.includes(pagesRoot.pathname); - // if this lives above src/pages, return that URL + const isPage = fileLoc.pathname.includes(pagesRoot.pathname) && (ext === '.astro' || ext === '.md'); + // if this lives in src/pages, return that URL if (isPage) { const [, publicURL] = projectLoc.split(pagesRoot.pathname); return publicURL || '/index.html'; // if this is missing, this is the root } + + // if this is in public/, use that as final URL + const isPublicAsset = fileLoc.pathname.includes(astroConfig.public.pathname); + if (isPublicAsset) { + return fileLoc.pathname.replace(astroConfig.public.pathname, '/'); + } + // otherwise, return /_astro/* url return '/_astro/' + projectLoc; } diff --git a/packages/astro/src/runtime.ts b/packages/astro/src/runtime.ts index a45b3fb82..e7f9efd1c 100644 --- a/packages/astro/src/runtime.ts +++ b/packages/astro/src/runtime.ts @@ -55,7 +55,7 @@ configureSnowpackLogger(snowpackLogger); /** Pass a URL to Astro to resolve and build */ async function load(config: RuntimeConfig, rawPathname: string | undefined): Promise { const { logging, snowpackRuntime, snowpack } = config; - const { pages: pagesRoot, buildOptions, devOptions } = config.astroConfig; + const { buildOptions, devOptions } = config.astroConfig; let origin = buildOptions.site ? new URL(buildOptions.site).origin : `http://localhost:${devOptions.port}`; const fullurl = new URL(rawPathname || '/', origin); @@ -63,7 +63,7 @@ async function load(config: RuntimeConfig, rawPathname: string | undefined): Pro const reqPath = decodeURI(fullurl.pathname); info(logging, 'access', reqPath); - const searchResult = searchForPage(fullurl, pagesRoot); + const searchResult = searchForPage(fullurl, config.astroConfig); if (searchResult.statusCode === 404) { try { const result = await snowpack.loadUrl(reqPath); @@ -332,7 +332,6 @@ async function createSnowpack(astroConfig: AstroConfig, options: CreateSnowpackO }; const mountOptions = { - [fileURLToPath(pagesRoot)]: '/_astro/pages', ...(existsSync(astroConfig.public) ? { [fileURLToPath(astroConfig.public)]: '/' } : {}), [fileURLToPath(frontendPath)]: '/_astro_frontend', [fileURLToPath(projectRoot)]: '/_astro', // must be last (greediest) diff --git a/packages/astro/src/search.ts b/packages/astro/src/search.ts index 233298991..198ec73b0 100644 --- a/packages/astro/src/search.ts +++ b/packages/astro/src/search.ts @@ -1,3 +1,5 @@ +import type { AstroConfig } from './@types/astro'; + import 'source-map-support/register.js'; import { existsSync } from 'fs'; import path from 'path'; @@ -9,13 +11,14 @@ interface PageLocation { snowpackURL: string; } /** findAnyPage and return the _astro candidate for snowpack */ -function findAnyPage(candidates: Array, pagesRoot: URL): PageLocation | false { +function findAnyPage(candidates: Array, astroConfig: AstroConfig): PageLocation | false { for (let candidate of candidates) { - const url = new URL(`./${candidate}`, pagesRoot); + const url = new URL(`./${candidate}`, astroConfig.pages); if (existsSync(url)) { + const pagesPath = astroConfig.pages.pathname.replace(astroConfig.projectRoot.pathname, ''); return { fileURL: url, - snowpackURL: `/_astro/pages/${candidate}.js`, + snowpackURL: `/_astro/${pagesPath}${candidate}.js`, }; } } @@ -39,14 +42,14 @@ type SearchResult = }; /** Given a URL, attempt to locate its source file (similar to Snowpack’s load()) */ -export function searchForPage(url: URL, pagesRoot: URL): SearchResult { +export function searchForPage(url: URL, astroConfig: AstroConfig): SearchResult { const reqPath = decodeURI(url.pathname); const base = reqPath.substr(1); // Try to find index.astro/md paths if (reqPath.endsWith('/')) { const candidates = [`${base}index.astro`, `${base}index.md`]; - const location = findAnyPage(candidates, pagesRoot); + const location = findAnyPage(candidates, astroConfig); if (location) { return { statusCode: 200, @@ -57,7 +60,7 @@ export function searchForPage(url: URL, pagesRoot: URL): SearchResult { } else { // Try to find the page by its name. const candidates = [`${base}.astro`, `${base}.md`]; - let location = findAnyPage(candidates, pagesRoot); + let location = findAnyPage(candidates, astroConfig); if (location) { return { statusCode: 200, @@ -69,7 +72,7 @@ export function searchForPage(url: URL, pagesRoot: URL): SearchResult { // Try to find name/index.astro/md const candidates = [`${base}/index.astro`, `${base}/index.md`]; - const location = findAnyPage(candidates, pagesRoot); + const location = findAnyPage(candidates, astroConfig); if (location) { return { statusCode: 301, @@ -81,7 +84,7 @@ export function searchForPage(url: URL, pagesRoot: URL): SearchResult { // Try and load collections (but only for non-extension files) const hasExt = !!path.extname(reqPath); if (!location && !hasExt) { - const collection = loadCollection(reqPath, pagesRoot); + const collection = loadCollection(reqPath, astroConfig); if (collection) { return { statusCode: 200, @@ -109,8 +112,8 @@ export function searchForPage(url: URL, pagesRoot: URL): SearchResult { } /** load a collection route */ -function loadCollection(url: string, pagesRoot: URL): { currentPage?: number; location: PageLocation } | undefined { - const pages = glob('**/$*.astro', { cwd: fileURLToPath(pagesRoot), filesOnly: true }); +function loadCollection(url: string, astroConfig: AstroConfig): { currentPage?: number; location: PageLocation } | undefined { + const pages = glob('**/$*.astro', { cwd: fileURLToPath(astroConfig.pages), filesOnly: true }); for (const pageURL of pages) { const reqURL = new RegExp('^/' + pageURL.replace(/\$([^/]+)\.astro/, '$1') + '/?(.*)'); const match = url.match(reqURL); @@ -125,10 +128,11 @@ function loadCollection(url: string, pagesRoot: URL): { currentPage?: number; lo } } } + const pagesPath = astroConfig.pages.pathname.replace(astroConfig.projectRoot.pathname, ''); return { location: { - fileURL: new URL(`./${pageURL}`, pagesRoot), - snowpackURL: `/_astro/pages/${pageURL}.js`, + fileURL: new URL(`./${pageURL}`, astroConfig.pages), + snowpackURL: `/_astro/${pagesPath}${pageURL}.js`, }, currentPage, }; diff --git a/packages/astro/test/astro-components.test.js b/packages/astro/test/astro-components.test.js index 7de88b8de..4f6fc05ab 100644 --- a/packages/astro/test/astro-components.test.js +++ b/packages/astro/test/astro-components.test.js @@ -6,7 +6,6 @@ import { setup, setupBuild } from './helpers.js'; const Components = suite('Components tests'); setup(Components, './fixtures/astro-components'); -setupBuild(Components, './fixtures/astro-components'); Components('Astro components are able to render framework components', async ({ runtime }) => { let result = await runtime.load('/'); diff --git a/packages/astro/test/astro-resolve.test.js b/packages/astro/test/astro-resolve.test.js new file mode 100644 index 000000000..ea9a307e6 --- /dev/null +++ b/packages/astro/test/astro-resolve.test.js @@ -0,0 +1,19 @@ +import { suite } from 'uvu'; +import * as assert from 'uvu/assert'; +import { setupBuild } from './helpers.js'; + +const Resolution = suite('Astro Resolution'); + +setupBuild(Resolution, './fixtures/astro-resolve'); + +Resolution('Assets', async (context) => { + await context.build(); + + // public/ asset resolved + assert.ok(await context.readFile('/svg.svg')); + + // asset in src/pages resolved (and didn’t overwrite /svg.svg) + assert.ok(await context.readFile('/_astro/src/pages/svg.svg')); +}); + +Resolution.run(); diff --git a/packages/astro/test/fixtures/astro-resolve/public/svg.svg b/packages/astro/test/fixtures/astro-resolve/public/svg.svg new file mode 100644 index 000000000..3e8fcacea --- /dev/null +++ b/packages/astro/test/fixtures/astro-resolve/public/svg.svg @@ -0,0 +1 @@ + diff --git a/packages/astro/test/fixtures/astro-resolve/snowpack.config.json b/packages/astro/test/fixtures/astro-resolve/snowpack.config.json new file mode 100644 index 000000000..8f034781d --- /dev/null +++ b/packages/astro/test/fixtures/astro-resolve/snowpack.config.json @@ -0,0 +1,3 @@ +{ + "workspaceRoot": "../../../../../" +} diff --git a/packages/astro/test/fixtures/astro-resolve/src/pages/index.astro b/packages/astro/test/fixtures/astro-resolve/src/pages/index.astro new file mode 100644 index 000000000..26f4bcf60 --- /dev/null +++ b/packages/astro/test/fixtures/astro-resolve/src/pages/index.astro @@ -0,0 +1,12 @@ + + + + + + + + + + + + diff --git a/packages/astro/test/fixtures/astro-resolve/src/pages/svg.svg b/packages/astro/test/fixtures/astro-resolve/src/pages/svg.svg new file mode 100644 index 000000000..3e8fcacea --- /dev/null +++ b/packages/astro/test/fixtures/astro-resolve/src/pages/svg.svg @@ -0,0 +1 @@ +