Improve asset resolution in Astro (#437)

* Improve asset resolution in Astro

Fixes #96

* Add docs, changeset

* Fix collection resolution

.changeset/loud-carrots-trade.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Improve asset resolution

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 `<img>` tag with a relative `src` attribute inside `src/pages/about.astro`:
+
+```html
+<!-- ❌ Incorrect: will try and load `/about/thumbnail.png` -->
+<img src="./thumbnail.png" />
+```
+
+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
+<!-- ✅ Correct: references public/thumbnail.png -->
+<img src="/thumbnail.png" />
+```
+
+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';
+---
+
+<img src={thumbnailSrc} />
+```
+
+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

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<void> {
   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',
+  };
 }

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;
 }

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<LoadResult> {
   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)

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<string>, pagesRoot: URL): PageLocation | false {
+function findAnyPage(candidates: Array<string>, 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,
       };

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('/');

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();

packages/astro/test/fixtures/astro-resolve/public/svg.svg

@@ -0,0 +1 @@
+<svg></svg>

packages/astro/test/fixtures/astro-resolve/snowpack.config.json

@@ -0,0 +1,3 @@
+{
+  "workspaceRoot": "../../../../../"
+}

packages/astro/test/fixtures/astro-resolve/src/pages/index.astro

@@ -0,0 +1,12 @@
+<html>
+  <head>
+  </head>
+
+  <body>
+    <!-- public/ -->
+    <img src="/svg.svg" />
+
+    <!-- src/ -->
+    <img src="./svg.svg" />
+  </body>
+</html>

packages/astro/test/fixtures/astro-resolve/src/pages/svg.svg

@@ -0,0 +1 @@
+<svg></svg>