diff --git a/.changeset/honest-snakes-peel.md b/.changeset/honest-snakes-peel.md new file mode 100644 index 000000000..c9cf064f6 --- /dev/null +++ b/.changeset/honest-snakes-peel.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix relative images in Markdown breaking the build process in certain circumstances diff --git a/.changeset/wise-donuts-tickle.md b/.changeset/wise-donuts-tickle.md new file mode 100644 index 000000000..38b1780ed --- /dev/null +++ b/.changeset/wise-donuts-tickle.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fix Astro HMR from a CSS dependency diff --git a/packages/astro/components/ViewTransitions.astro b/packages/astro/components/ViewTransitions.astro index 39b9996ce..aa266af13 100644 --- a/packages/astro/components/ViewTransitions.astro +++ b/packages/astro/components/ViewTransitions.astro @@ -17,18 +17,26 @@ const { fallback = 'animate' } = Astro.props as Props; index: number; scrollX: number; scrollY: number; + intraPage?: boolean; }; type Events = 'astro:page-load' | 'astro:after-swap'; // only update history entries that are managed by us // leave other entries alone and do not accidently add state. const persistState = (state: State) => history.state && history.replaceState(state, ''); + // @ts-expect-error: startViewTransition might exist const supportsViewTransitions = !!document.startViewTransition; const transitionEnabledOnThisPage = () => !!document.querySelector('[name="astro-view-transitions-enabled"]'); const triggerEvent = (name: Events) => document.dispatchEvent(new Event(name)); const onPageLoad = () => triggerEvent('astro:page-load'); const PERSIST_ATTR = 'data-astro-transition-persist'; + const parser = new DOMParser(); + // explained at its usage + let noopEl: HTMLDivElement; + if (import.meta.env.DEV) { + noopEl = document.createElement('div'); + } // The History API does not tell you if navigation is forward or back, so // you can figure it using an index. On pushState the index is incremented so you @@ -40,7 +48,7 @@ const { fallback = 'animate' } = Astro.props as Props; currentHistoryIndex = history.state.index; scrollTo({ left: history.state.scrollX, top: history.state.scrollY }); } else if (transitionEnabledOnThisPage()) { - history.replaceState({ index: currentHistoryIndex, scrollX, scrollY }, ''); + history.replaceState({ index: currentHistoryIndex, scrollX, scrollY, intraPage: false }, ''); } const throttle = (cb: (...args: any[]) => any, delay: number) => { let wait = false; @@ -64,19 +72,28 @@ const { fallback = 'animate' } = Astro.props as Props; }; }; - async function getHTML(href: string) { + // returns the contents of the page or null if the router can't deal with it. + async function fetchHTML( + href: string + ): Promise { try { const res = await fetch(href); + // drop potential charset (+ other name/value pairs) as parser needs the mediaType + const mediaType = res.headers.get('content-type')?.replace(/;.*$/, ''); + // the DOMParser can handle two types of HTML + if (mediaType !== 'text/html' && mediaType !== 'application/xhtml+xml') { + // everything else (e.g. audio/mp3) will be handled by the browser but not by us + return null; + } const html = await res.text(); return { - ok: res.ok, html, redirected: res.redirected ? res.url : undefined, - // drop potential charset (+ other name/value pairs) as parser needs the mediaType - mediaType: res.headers.get('content-type')?.replace(/;.*$/, ''), + mediaType, }; } catch (err) { - return { ok: false }; + // can't fetch, let someone else deal with it. + return null; } } @@ -98,19 +115,19 @@ const { fallback = 'animate' } = Astro.props as Props; let wait = Promise.resolve(); for (const script of Array.from(document.scripts)) { if (script.dataset.astroExec === '') continue; - const s = document.createElement('script'); - s.innerHTML = script.innerHTML; + const newScript = document.createElement('script'); + newScript.innerHTML = script.innerHTML; for (const attr of script.attributes) { if (attr.name === 'src') { const p = new Promise((r) => { - s.onload = r; + newScript.onload = r; }); wait = wait.then(() => p as any); } - s.setAttribute(attr.name, attr.value); + newScript.setAttribute(attr.name, attr.value); } - s.dataset.astroExec = ''; - script.replaceWith(s); + newScript.dataset.astroExec = ''; + script.replaceWith(newScript); } return wait; } @@ -122,16 +139,39 @@ const { fallback = 'animate' } = Astro.props as Props; return style.animationIterationCount === 'infinite'; } - const parser = new DOMParser(); + const updateHistoryAndScrollPosition = (toLocation) => { + if (toLocation.href !== location.href) { + history.pushState( + { index: ++currentHistoryIndex, scrollX: 0, scrollY: 0 }, + '', + toLocation.href + ); + // now we are on the new page for non-history navigations! + // (with history navigation page change happens before popstate is fired) + } + // freshly loaded pages start from the top + scrollTo({ left: 0, top: 0, behavior: 'instant' }); - // A noop element used to prevent styles from being removed - if (import.meta.env.DEV) { - var noopEl = document.createElement('div'); - } + if (toLocation.hash) { + // because we are already on the target page ... + // ... what comes next is a intra-page navigation + // that won't reload the page but instead scroll to the fragment + location.href = toLocation.href; + } + }; - async function updateDOM(newDocument: Document, loc: URL, state?: State, fallback?: Fallback) { - // Check for a head element that should persist, either because it has the data - // attribute or is a link el. + // replace head and body of the windows document with contents from newDocument + // if !popstate, update the history entry and scroll position according to toLocation + // if popState is given, this holds the scroll position for history navigation + // if fallback === "animate" then simulate view transitions + async function updateDOM( + newDocument: Document, + toLocation: URL, + popState?: State, + fallback?: Fallback + ) { + // Check for a head element that should persist and returns it, + // either because it has the data attribute or is a link el. const persistedHeadElement = (el: HTMLElement): Element | null => { const id = el.getAttribute(PERSIST_ATTR); const newEl = id && newDocument.head.querySelector(`[${PERSIST_ATTR}="${id}"]`); @@ -142,7 +182,11 @@ const { fallback = 'animate' } = Astro.props as Props; const href = el.getAttribute('href'); return newDocument.head.querySelector(`link[rel=stylesheet][href="${href}"]`); } - // Only run this in dev. This will get stripped from production builds and is not needed. + // What follows is a fix for an issue (#8472) with missing client:only styles after transition. + // That problem exists only in dev mode where styles are injected into the page by Vite. + // Returning a noop element ensures that the styles are not removed from the old document. + // Guarding the code below with the dev mode check + // allows tree shaking to remove this code in production. if (import.meta.env.DEV) { if (el.tagName === 'STYLE' && el.dataset.viteDevId) { const devId = el.dataset.viteDevId; @@ -158,10 +202,6 @@ const { fallback = 'animate' } = Astro.props as Props; }; const swap = () => { - // noscript tags inside head element are not honored on swap (#7969). - // Remove them before swapping. - newDocument.querySelectorAll('head noscript').forEach((el) => el.remove()); - // swap attributes of the html element // - delete all attributes from the current document // - insert all attributes from doc @@ -208,6 +248,8 @@ const { fallback = 'animate' } = Astro.props as Props; // Persist elements in the existing body const oldBody = document.body; + + // this will reset scroll Position document.body.replaceWith(newDocument.body); for (const el of oldBody.querySelectorAll(`[${PERSIST_ATTR}]`)) { const id = el.getAttribute(PERSIST_ATTR); @@ -219,33 +261,12 @@ const { fallback = 'animate' } = Astro.props as Props; } } - // Simulate scroll behavior of Safari and - // Chromium based browsers (Chrome, Edge, Opera, ...) - scrollTo({ left: 0, top: 0, behavior: 'instant' }); - - let initialScrollX = 0; - let initialScrollY = 0; - if (!state && loc.hash) { - const id = decodeURIComponent(loc.hash.slice(1)); - const elem = document.getElementById(id); - // prefer scrollIntoView() over scrollTo() because it takes scroll-padding into account - if (elem) { - elem.scrollIntoView(); - initialScrollX = Math.max( - 0, - elem.offsetLeft + elem.offsetWidth - document.documentElement.clientWidth - ); - initialScrollY = elem.offsetTop; - } - } else if (state) { - scrollTo(state.scrollX, state.scrollY); // usings default scrollBehavior + if (popState) { + scrollTo(popState.scrollX, popState.scrollY); // usings 'auto' scrollBehavior + } else { + updateHistoryAndScrollPosition(toLocation); } - !state && - history.pushState( - { index: ++currentHistoryIndex, scrollX: initialScrollX, scrollY: initialScrollY }, - '', - loc.href - ); + triggerEvent('astro:after-swap'); }; @@ -291,32 +312,44 @@ const { fallback = 'animate' } = Astro.props as Props; } } - async function navigate(dir: Direction, loc: URL, state?: State) { + async function transition(direction: Direction, toLocation: URL, popState?: State) { let finished: Promise; - const href = loc.href; - const { html, ok, mediaType, redirected } = await getHTML(href); - // if there was a redirection, show the final URL in the browser's address bar - redirected && (loc = new URL(redirected)); + const href = toLocation.href; + const response = await fetchHTML(href); // If there is a problem fetching the new page, just do an MPA navigation to it. - if (!ok || !(mediaType === 'text/html' || mediaType === 'application/xhtml+xml')) { + if (response === null) { location.href = href; return; } + // if there was a redirection, show the final URL in the browser's address bar + if (response.redirected) { + toLocation = new URL(response.redirected); + } + + const newDocument = parser.parseFromString(response.html, response.mediaType); + // The next line might look like a hack, + // but it is actually necessary as noscript elements + // and their contents are returned as markup by the parser, + // see https://developer.mozilla.org/en-US/docs/Web/API/DOMParser/parseFromString + newDocument.querySelectorAll('noscript').forEach((el) => el.remove()); - const newDocument = parser.parseFromString(html, mediaType); if (!newDocument.querySelector('[name="astro-view-transitions-enabled"]')) { location.href = href; return; } - // Now we are sure that we will push state, and it is time to create a state if it is still missing. - !state && history.replaceState({ index: currentHistoryIndex, scrollX, scrollY }, ''); - - document.documentElement.dataset.astroTransition = dir; + if (!popState) { + // save the current scroll position before we change the DOM and transition to the new page + history.replaceState({ ...history.state, scrollX, scrollY }, ''); + } + document.documentElement.dataset.astroTransition = direction; if (supportsViewTransitions) { - finished = document.startViewTransition(() => updateDOM(newDocument, loc, state)).finished; + // @ts-expect-error: startViewTransition exist + finished = document.startViewTransition(() => + updateDOM(newDocument, toLocation, popState) + ).finished; } else { - finished = updateDOM(newDocument, loc, state, getFallback()); + finished = updateDOM(newDocument, toLocation, popState, getFallback()); } try { await finished; @@ -332,7 +365,9 @@ const { fallback = 'animate' } = Astro.props as Props; // Prefetching function maybePrefetch(pathname: string) { if (document.querySelector(`link[rel=prefetch][href="${pathname}"]`)) return; + // @ts-expect-error: connection might exist if (navigator.connection) { + // @ts-expect-error: connection does exist let conn = navigator.connection; if (conn.saveData || /(2|3)g/.test(conn.effectiveType || '')) return; } @@ -343,8 +378,6 @@ const { fallback = 'animate' } = Astro.props as Props; } if (supportsViewTransitions || getFallback() !== 'none') { - markScriptsExec(); - document.addEventListener('click', (ev) => { let link = ev.target; if (link instanceof Element && link.tagName !== 'A') { @@ -366,51 +399,59 @@ const { fallback = 'animate' } = Astro.props as Props; ev.ctrlKey || // new tab (windows) ev.altKey || // download ev.shiftKey || // new window - ev.defaultPrevented || - !transitionEnabledOnThisPage() + ev.defaultPrevented ) { // No page transitions in these cases, // Let the browser standard action handle this return; } - // We do not need to handle same page links because there are no page transitions - // Same page means same path and same query params (but different hash) - if (location.pathname === link.pathname && location.search === link.search) { - if (link.hash) { - // The browser default action will handle navigations with hash fragments - return; - } else { - // Special case: self link without hash - // If handed to the browser it will reload the page - // But we want to handle it like any other same page navigation - // So we scroll to the top of the page but do not start page transitions - ev.preventDefault(); - // push state on the first navigation but not if we were here already - if (location.hash) { - history.replaceState( - { index: currentHistoryIndex, scrollX, scrollY: -(scrollY + 1) }, - '' - ); - const newState: State = { index: ++currentHistoryIndex, scrollX: 0, scrollY: 0 }; - history.pushState(newState, '', link.href); - } - scrollTo({ left: 0, top: 0, behavior: 'instant' }); - return; - } - } - - // these are the cases we will handle: same origin, different page ev.preventDefault(); - navigate('forward', new URL(link.href)); + navigate(link.href); }); + function navigate(href) { + // not ours + if (!transitionEnabledOnThisPage()) { + location.href = href; + return; + } + const toLocation = new URL(href, location.href); + // We do not have page transitions on navigations to the same page (intra-page navigation) + // but we want to handle prevent reload on navigation to the same page + // Same page means same origin, path and query params (but maybe different hash) + if ( + location.origin === toLocation.origin && + location.pathname === toLocation.pathname && + location.search === toLocation.search + ) { + // mark current position as non transition intra-page scrolling + if (location.href !== toLocation.href) { + history.replaceState({ ...history.state, intraPage: true }, ''); + history.pushState( + { index: ++currentHistoryIndex, scrollX: 0, scrollY: 0 }, + '', + toLocation.href + ); + } + if (toLocation.hash) { + location.href = toLocation.href; + } else { + scrollTo({ left: 0, top: 0, behavior: 'instant' }); + } + } else { + transition('forward', toLocation); + } + } + addEventListener('popstate', (ev) => { if (!transitionEnabledOnThisPage() && ev.state) { // The current page doesn't have View Transitions enabled // but the page we navigate to does (because it set the state). // Do a full page refresh to reload the client-side router from the new page. // Scroll restauration will then happen during the reload when the router's code is re-executed - history.scrollRestoration && (history.scrollRestoration = 'manual'); + if (history.scrollRestoration) { + history.scrollRestoration = 'manual'; + } location.reload(); return; } @@ -433,13 +474,14 @@ const { fallback = 'animate' } = Astro.props as Props; } const state: State = history.state; - const nextIndex = state.index; - const direction: Direction = nextIndex > currentHistoryIndex ? 'forward' : 'back'; - currentHistoryIndex = nextIndex; - if (state.scrollY < 0) { - scrollTo(state.scrollX, -(state.scrollY + 1)); + if (state.intraPage) { + // this is non transition intra-page scrolling + scrollTo(state.scrollX, state.scrollY); } else { - navigate(direction, new URL(location.href), state); + const nextIndex = state.index; + const direction: Direction = nextIndex > currentHistoryIndex ? 'forward' : 'back'; + currentHistoryIndex = nextIndex; + transition(direction, new URL(location.href), state); } }); @@ -461,6 +503,7 @@ const { fallback = 'animate' } = Astro.props as Props; { passive: true, capture: true } ); }); + addEventListener('load', onPageLoad); // There's not a good way to record scroll position before a back button. // So the way we do it is by listening to scrollend if supported, and if not continuously record the scroll position. @@ -470,5 +513,7 @@ const { fallback = 'animate' } = Astro.props as Props; if ('onscrollend' in window) addEventListener('scrollend', updateState); else addEventListener('scroll', throttle(updateState, 300)); + + markScriptsExec(); } diff --git a/packages/astro/e2e/astro-component.test.js b/packages/astro/e2e/astro-component.test.js index 7308ea292..c96e9b1c4 100644 --- a/packages/astro/e2e/astro-component.test.js +++ b/packages/astro/e2e/astro-component.test.js @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import { getColor, testFactory } from './test-utils.js'; +import { testFactory } from './test-utils.js'; const test = testFactory({ root: './fixtures/astro-component/' }); @@ -99,7 +99,7 @@ test.describe('Astro component HMR', () => { test('update linked dep Astro style', async ({ page, astro }) => { await page.goto(astro.resolveUrl('/')); let h1 = page.locator('#astro-linked-lib'); - expect(await getColor(h1)).toBe('rgb(255, 0, 0)'); + await expect(h1).toHaveCSS('color', 'rgb(255, 0, 0)'); await Promise.all([ page.waitForLoadState('networkidle'), await astro.editFile('../_deps/astro-linked-lib/Component.astro', (content) => @@ -107,6 +107,6 @@ test.describe('Astro component HMR', () => { ), ]); h1 = page.locator('#astro-linked-lib'); - expect(await getColor(h1)).toBe('rgb(0, 128, 0)'); + await expect(h1).toHaveCSS('color', 'rgb(0, 128, 0)'); }); }); diff --git a/packages/astro/e2e/css.test.js b/packages/astro/e2e/css.test.js index 184e5dba3..b302d9d90 100644 --- a/packages/astro/e2e/css.test.js +++ b/packages/astro/e2e/css.test.js @@ -1,5 +1,5 @@ import { expect } from '@playwright/test'; -import { getColor, testFactory } from './test-utils.js'; +import { testFactory } from './test-utils.js'; const test = testFactory({ root: './fixtures/css/', @@ -20,13 +20,13 @@ test.describe('CSS HMR', () => { await page.goto(astro.resolveUrl('/')); const h = page.locator('h1'); - expect(await getColor(h)).toBe('rgb(255, 0, 0)'); + await expect(h).toHaveCSS('color', 'rgb(255, 0, 0)'); await astro.editFile('./src/styles/main.css', (original) => original.replace('--h1-color: red;', '--h1-color: green;') ); - expect(await getColor(h)).toBe('rgb(0, 128, 0)'); + await expect(h).toHaveCSS('color', 'rgb(0, 128, 0)'); }); test('removes Astro-injected CSS once Vite-injected CSS loads', async ({ page, astro }) => { diff --git a/packages/astro/e2e/fixtures/invalidate-script-deps/package.json b/packages/astro/e2e/fixtures/hmr/package.json similarity index 50% rename from packages/astro/e2e/fixtures/invalidate-script-deps/package.json rename to packages/astro/e2e/fixtures/hmr/package.json index 4b45ad505..f5aa41460 100644 --- a/packages/astro/e2e/fixtures/invalidate-script-deps/package.json +++ b/packages/astro/e2e/fixtures/hmr/package.json @@ -1,8 +1,9 @@ { - "name": "@e2e/invalidate-script-deps", + "name": "@e2e/hmr", "version": "0.0.0", "private": true, "devDependencies": { - "astro": "workspace:*" + "astro": "workspace:*", + "sass": "^1.66.1" } } diff --git a/packages/astro/e2e/fixtures/hmr/src/pages/css-dep.astro b/packages/astro/e2e/fixtures/hmr/src/pages/css-dep.astro new file mode 100644 index 000000000..38f4e7409 --- /dev/null +++ b/packages/astro/e2e/fixtures/hmr/src/pages/css-dep.astro @@ -0,0 +1,14 @@ + + + Test + + +

This is blue

+ + + diff --git a/packages/astro/e2e/fixtures/invalidate-script-deps/src/pages/index.astro b/packages/astro/e2e/fixtures/hmr/src/pages/script-dep.astro similarity index 100% rename from packages/astro/e2e/fixtures/invalidate-script-deps/src/pages/index.astro rename to packages/astro/e2e/fixtures/hmr/src/pages/script-dep.astro diff --git a/packages/astro/e2e/fixtures/invalidate-script-deps/src/scripts/heading.js b/packages/astro/e2e/fixtures/hmr/src/scripts/heading.js similarity index 100% rename from packages/astro/e2e/fixtures/invalidate-script-deps/src/scripts/heading.js rename to packages/astro/e2e/fixtures/hmr/src/scripts/heading.js diff --git a/packages/astro/e2e/fixtures/hmr/src/styles/vars.scss b/packages/astro/e2e/fixtures/hmr/src/styles/vars.scss new file mode 100644 index 000000000..5deae109f --- /dev/null +++ b/packages/astro/e2e/fixtures/hmr/src/styles/vars.scss @@ -0,0 +1 @@ +$color: blue; \ No newline at end of file diff --git a/packages/astro/e2e/invalidate-script-deps.test.js b/packages/astro/e2e/hmr.test.js similarity index 57% rename from packages/astro/e2e/invalidate-script-deps.test.js rename to packages/astro/e2e/hmr.test.js index fe37ece8f..091aa716d 100644 --- a/packages/astro/e2e/invalidate-script-deps.test.js +++ b/packages/astro/e2e/hmr.test.js @@ -2,7 +2,7 @@ import { expect } from '@playwright/test'; import { testFactory } from './test-utils.js'; const test = testFactory({ - root: './fixtures/invalidate-script-deps/', + root: './fixtures/hmr/', }); let devServer; @@ -17,7 +17,7 @@ test.afterAll(async () => { test.describe('Scripts with dependencies', () => { test('refresh with HMR', async ({ page, astro }) => { - await page.goto(astro.resolveUrl('/')); + await page.goto(astro.resolveUrl('/script-dep')); const h = page.locator('h1'); await expect(h, 'original text set').toHaveText('before'); @@ -29,3 +29,16 @@ test.describe('Scripts with dependencies', () => { await expect(h, 'text changed').toHaveText('after'); }); }); + +test.describe('Styles with dependencies', () => { + test('refresh with HMR', async ({ page, astro }) => { + await page.goto(astro.resolveUrl('/css-dep')); + + const h = page.locator('h1'); + await expect(h).toHaveCSS('color', 'rgb(0, 0, 255)'); + + await astro.editFile('./src/styles/vars.scss', (original) => original.replace('blue', 'red')); + + await expect(h).toHaveCSS('color', 'rgb(255, 0, 0)'); + }); +}); diff --git a/packages/astro/e2e/test-utils.js b/packages/astro/e2e/test-utils.js index 0768bff81..79b7601b7 100644 --- a/packages/astro/e2e/test-utils.js +++ b/packages/astro/e2e/test-utils.js @@ -71,13 +71,6 @@ export async function getErrorOverlayContent(page) { return { message, hint, absoluteFileLocation, fileLocation }; } -/** - * @returns {Promise} - */ -export async function getColor(el) { - return await el.evaluate((e) => getComputedStyle(e).color); -} - /** * Wait for `astro-island` that contains the `el` to hydrate * @param {import('@playwright/test').Page} page diff --git a/packages/astro/e2e/view-transitions.test.js b/packages/astro/e2e/view-transitions.test.js index d777d9b2f..b06d5a988 100644 --- a/packages/astro/e2e/view-transitions.test.js +++ b/packages/astro/e2e/view-transitions.test.js @@ -293,12 +293,12 @@ test.describe('View Transitions', () => { locator = page.locator('#click-one-again'); await expect(locator).toBeInViewport(); - // Scroll up to top fragment + // goto page 1 await page.click('#click-one-again'); locator = page.locator('#one'); await expect(locator).toHaveText('Page 1'); - // Back to middle of the page + // Back to middle of the previous page await page.goBack(); locator = page.locator('#click-one-again'); await expect(locator).toBeInViewport(); diff --git a/packages/astro/src/assets/services/squoosh.ts b/packages/astro/src/assets/services/squoosh.ts index 4ba78f8d3..95c16b8d8 100644 --- a/packages/astro/src/assets/services/squoosh.ts +++ b/packages/astro/src/assets/services/squoosh.ts @@ -29,8 +29,11 @@ const qualityTable: Record< // Squoosh's PNG encoder does not support a quality setting, so we can skip that here }; -async function getRotationForEXIF(inputBuffer: Buffer): Promise { - const meta = await imageMetadata(inputBuffer); +async function getRotationForEXIF( + inputBuffer: Buffer, + src?: string +): Promise { + const meta = await imageMetadata(inputBuffer, src); if (!meta) return undefined; // EXIF orientations are a bit hard to read, but the numbers are actually standard. See https://exiftool.org/TagNames/EXIF.html for a list. @@ -63,7 +66,7 @@ const service: LocalImageService = { const operations: Operation[] = []; - const rotation = await getRotationForEXIF(inputBuffer); + const rotation = await getRotationForEXIF(inputBuffer, transform.src); if (rotation) { operations.push(rotation); diff --git a/packages/astro/src/assets/utils/emitAsset.ts b/packages/astro/src/assets/utils/emitAsset.ts index 9b83a020a..b9ca146b7 100644 --- a/packages/astro/src/assets/utils/emitAsset.ts +++ b/packages/astro/src/assets/utils/emitAsset.ts @@ -22,11 +22,7 @@ export async function emitESMImage( return undefined; } - const fileMetadata = await imageMetadata(fileData); - - if (!fileMetadata) { - return undefined; - } + const fileMetadata = await imageMetadata(fileData, id); const emittedImage: ImageMetadata = { src: '', diff --git a/packages/astro/src/assets/utils/metadata.ts b/packages/astro/src/assets/utils/metadata.ts index 7d7ee7457..fc89ca1ca 100644 --- a/packages/astro/src/assets/utils/metadata.ts +++ b/packages/astro/src/assets/utils/metadata.ts @@ -1,19 +1,23 @@ import probe from 'probe-image-size'; +import { AstroError, AstroErrorData } from '../../core/errors/index.js'; import type { ImageInputFormat, ImageMetadata } from '../types.js'; -export async function imageMetadata(data: Buffer): Promise | undefined> { +export async function imageMetadata( + data: Buffer, + src?: string +): Promise> { const result = probe.sync(data); + if (result === null) { - throw new Error('Failed to probe image size.'); + throw new AstroError({ + ...AstroErrorData.NoImageMetadata, + message: AstroErrorData.NoImageMetadata.message(src), + }); } const { width, height, type, orientation } = result; const isPortrait = (orientation || 0) >= 5; - if (!width || !height || !type) { - return undefined; - } - return { width: isPortrait ? height : width, height: isPortrait ? width : height, diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index 0fe45d1ab..9c95b6dc4 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -2,6 +2,8 @@ import MagicString from 'magic-string'; import type * as vite from 'vite'; import { normalizePath } from 'vite'; import type { AstroPluginOptions, ImageTransform } from '../@types/astro.js'; +import { extendManualChunks } from '../core/build/plugins/util.js'; +import { AstroError, AstroErrorData } from '../core/errors/index.js'; import { appendForwardSlash, joinPaths, @@ -28,6 +30,18 @@ export default function assets({ // Expose the components and different utilities from `astro:assets` and handle serving images from `/_image` in dev { name: 'astro:assets', + outputOptions(outputOptions) { + // Specifically split out chunk for asset files to prevent TLA deadlock + // caused by `getImage()` for markdown components. + // https://github.com/rollup/rollup/issues/4708 + extendManualChunks(outputOptions, { + after(id) { + if (id.includes('astro/dist/assets/services/')) { + return `astro-assets-services`; + } + }, + }); + }, async resolveId(id) { if (id === VIRTUAL_SERVICE_ID) { return await this.resolve(settings.config.image.service.entrypoint); @@ -125,6 +139,14 @@ export default function assets({ } if (assetRegex.test(id)) { const meta = await emitESMImage(id, this.meta.watchMode, this.emitFile); + + if (!meta) { + throw new AstroError({ + ...AstroErrorData.ImageNotFound, + message: AstroErrorData.ImageNotFound.message(id), + }); + } + return `export default ${JSON.stringify(meta)}`; } }, diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index 1f336e5f8..e4fe35540 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -620,8 +620,42 @@ export const ExpectedImageOptions = { message: (options: string) => `Expected getImage() parameter to be an object. Received \`${options}\`.`, } satisfies ErrorData; + /** * @docs + * @see + * - [Images](https://docs.astro.build/en/guides/images/) + * @description + * Astro could not find an image you imported. Often, this is simply caused by a typo in the path. + * + * Images in Markdown are relative to the current file. To refer to an image that is located in the same folder as the `.md` file, the path should start with `./` + */ +export const ImageNotFound = { + name: 'ImageNotFound', + title: 'Image not found.', + message: (imagePath: string) => `Could not find requested image \`${imagePath}\`. Does it exist?`, + hint: 'This is often caused by a typo in the image path. Please make sure the file exists, and is spelled correctly.', +} satisfies ErrorData; + +/** + * @docs + * @message Could not process image metadata for `IMAGE_PATH`. + * @see + * - [Images](https://docs.astro.build/en/guides/images/) + * @description + * Astro could not process the metadata of an image you imported. This is often caused by a corrupted or malformed image and re-exporting the image from your image editor may fix this issue. + */ +export const NoImageMetadata = { + name: 'NoImageMetadata', + title: 'Could not process image metadata.', + message: (imagePath: string | undefined) => + `Could not process image metadata${imagePath ? ' for `${imagePath}`' : ''}.`, + hint: 'This is often caused by a corrupted or malformed image. Re-exporting the image from your image editor may fix this issue.', +} satisfies ErrorData; + +/** + * @docs + * @deprecated This error is no longer Markdown specific and as such, as been replaced by `ImageNotFound` * @message * Could not find requested image `IMAGE_PATH` at `FULL_IMAGE_PATH`. * @see @@ -640,6 +674,7 @@ export const MarkdownImageNotFound = { }`, hint: 'This is often caused by a typo in the image path. Please make sure the file exists, and is spelled correctly.', } satisfies ErrorData; + /** * @docs * @description diff --git a/packages/astro/src/vite-plugin-astro/hmr.ts b/packages/astro/src/vite-plugin-astro/hmr.ts index 65186af5e..6600b2f42 100644 --- a/packages/astro/src/vite-plugin-astro/hmr.ts +++ b/packages/astro/src/vite-plugin-astro/hmr.ts @@ -90,7 +90,7 @@ export async function handleHotUpdate( // Bugfix: sometimes style URLs get normalized and end with `lang.css=` // These will cause full reloads, so filter them out here - const mods = ctx.modules.filter((m) => !m.url.endsWith('=')); + const mods = [...filtered].filter((m) => !m.url.endsWith('=')); const file = ctx.file.replace(config.root.pathname, '/'); // If only styles are changed, remove the component file from the update list @@ -109,17 +109,6 @@ export async function handleHotUpdate( } } - // If this is a module that is imported from a