Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into feat/redirect-default…
Browse files Browse the repository at this point in the history
…-language
  • Loading branch information
ematipico committed Jan 16, 2024
2 parents e3255cb + 39050c6 commit a5fd580
Show file tree
Hide file tree
Showing 26 changed files with 208 additions and 44 deletions.
5 changes: 5 additions & 0 deletions .changeset/friendly-needles-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Improves HMR for Astro style and script modules
5 changes: 5 additions & 0 deletions .changeset/poor-cherries-buy.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Disables View Transition form handling when the `action` property points to an external URL
5 changes: 5 additions & 0 deletions .changeset/slimy-mayflies-vanish.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fix Astro wrongfully deleting certain images imported with `?url` when used in tandem with `astro:assets`
4 changes: 4 additions & 0 deletions .changeset/weak-planes-help.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
"astro": patch
---
Fixes an issue where anchor elements within a custom component could not trigger a view transition.
26 changes: 15 additions & 11 deletions .github/workflows/check-merge.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,26 @@ jobs:
files: |
.changeset/**/*.md
- name: Check if any changesets contain minor changes
id: minor
- name: Check if any changesets contain minor or major changes
id: check
if: steps.blocked.outputs.result != 'true'
run: |
echo "Checking for changesets marked as minor"
echo "Checking for changesets marked as minor or major"
echo "found=false" >> $GITHUB_OUTPUT
regex="[\"']astro[\"']: (minor|major)"
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
if grep -q "'astro': minor" "$file"; then
echo "found=true" >> $GITHUB_OUTPUT
echo "$file has a minor release tag"
fi
if [[ $(cat $file) =~ $regex ]]; then
version="${BASH_REMATCH[1]}"
echo "version=$version" >> $GITHUB_OUTPUT
echo "found=true" >> $GITHUB_OUTPUT
echo "$file has a $version release tag"
fi
done
- name: Add label
uses: actions/github-script@v6
if: steps.minor.outputs.found == 'true'
if: steps.check.outputs.found == 'true'
env:
issue_number: ${{ github.event.number }}
with:
Expand All @@ -69,12 +73,12 @@ jobs:
issue_number: process.env.issue_number,
owner: context.repo.owner,
repo: context.repo.repo,
labels: ['semver: minor']
labels: ['semver: ${{ steps.check.outputs.version }}']
});
- name: Change PR Status
uses: actions/github-script@v6
if: steps.minor.outputs.found == 'true'
if: steps.check.outputs.found == 'true'
env:
issue_number: ${{ github.event.number }}
with:
Expand All @@ -84,5 +88,5 @@ jobs:
repo: context.repo.repo,
pull_number: process.env.issue_number,
event: 'REQUEST_CHANGES',
body: 'This PR is blocked because it contains a `minor` changeset. A reviewer will merge this at the next release if approved.'
body: 'This PR is blocked because it contains a `${{ steps.check.outputs.version }}` changeset. A reviewer will merge this at the next release if approved.'
});
7 changes: 6 additions & 1 deletion packages/astro/components/ViewTransitions.astro
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ const { fallback = 'animate' } = Astro.props;
if (supportsViewTransitions || getFallback() !== 'none') {
document.addEventListener('click', (ev) => {
let link = ev.target;
if (ev.composed) {
link = ev.composedPath()[0];
}
if (link instanceof Element) {
link = link.closest('a, area');
}
Expand Down Expand Up @@ -106,7 +109,9 @@ const { fallback = 'animate' } = Astro.props;

// the "dialog" method is a special keyword used within <dialog> elements
// https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#attr-fs-method
if (method === 'dialog') {
if (method === 'dialog' || location.origin !== new URL(action, location.href).origin) {
// No page transitions in these cases,
// Let the browser standard action handle this
return;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
import Layout from '../components/Layout.astro';
---
<Layout>
<form action="https://example.com/" method="POST">
<button id="submit">Submit</button>
</form>
</Layout>
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ import Layout from '../components/Layout.astro';
<a id="click-redirect-two" href="/redirect-two">go to redirect 2</a>
<a id="click-redirect-external" href="/redirect-external">go to a redirect external</a>
<a id="click-404" href="/undefined-page">go to undefined page</a>
<custom-a id="custom-click-two">
<template shadowrootmode="open">
<a href="/two">go to 2</a>
</template>
</custom-a>

<div id="test">test content</div>
</Layout>
21 changes: 21 additions & 0 deletions packages/astro/e2e/view-transitions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,13 @@ test.describe('View Transitions', () => {
).toEqual(1);
});

test('form POST that action for cross-origin is opt-out', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/form-five'));
page.on('request', (request) => expect(request.method()).toBe('POST'));
// Submit the form
await page.click('#submit');
});

test('form GET that redirects to another page is handled', async ({ page, astro }) => {
const loads = [];
page.addListener('load', async (p) => {
Expand Down Expand Up @@ -1206,4 +1213,18 @@ test.describe('View Transitions', () => {

expect(loads.length, 'There should only be 1 page load').toEqual(1);
});

test('custom elements can trigger a view transition', async ({ page, astro }) => {
const loads = [];
page.addListener('load', (p) => {
loads.push(p.title());
});
await page.goto(astro.resolveUrl('/one'));
await expect(page.locator('#one'), 'should have content').toHaveText('Page 1');
// go to page 2
await page.click('#custom-click-two');
await expect(page.locator('#two'), 'should have content').toHaveText('Page 2');

expect(loads.length, 'There should only be 1 page load').toEqual(1);
});
});
8 changes: 6 additions & 2 deletions packages/astro/src/assets/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ export async function getImage(
: options.src,
};

const originalPath = isESMImportedImage(resolvedOptions.src)
? resolvedOptions.src.fsPath
: resolvedOptions.src;

// Clone the `src` object if it's an ESM import so that we don't refer to any properties of the original object
// Causing our generate step to think the image is used outside of the image optimization pipeline
const clonedSrc = isESMImportedImage(resolvedOptions.src)
Expand Down Expand Up @@ -95,10 +99,10 @@ export async function getImage(
!(isRemoteImage(validatedOptions.src) && imageURL === validatedOptions.src)
) {
const propsToHash = service.propertiesToHash ?? DEFAULT_HASH_PROPS;
imageURL = globalThis.astroAsset.addStaticImage(validatedOptions, propsToHash);
imageURL = globalThis.astroAsset.addStaticImage(validatedOptions, propsToHash, originalPath);
srcSets = srcSetTransforms.map((srcSet) => ({
transform: srcSet.transform,
url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash),
url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash, originalPath),
descriptor: srcSet.descriptor,
attributes: srcSet.attributes,
}));
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/assets/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ declare global {
// eslint-disable-next-line no-var
var astroAsset: {
imageService?: ImageService;
addStaticImage?: ((options: ImageTransform, hashProperties: string[]) => string) | undefined;
addStaticImage?:
| ((options: ImageTransform, hashProperties: string[], fsPath: string) => string)
| undefined;
staticImages?: AssetsGlobalStaticImagesList;
referencedImages?: Set<string>;
};
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/assets/utils/emitAsset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export async function emitESMImage(
Object.defineProperty(emittedImage, 'fsPath', {
enumerable: false,
writable: false,
value: url,
value: id,
});

// Build
Expand Down
10 changes: 8 additions & 2 deletions packages/astro/src/assets/utils/proxy.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
export function getProxyCode(options: Record<string, any>, isSSR: boolean): string {
import type { ImageMetadata } from '../types.js';

export function getProxyCode(options: ImageMetadata, isSSR: boolean): string {
const stringifiedFSPath = JSON.stringify(options.fsPath);
return `
new Proxy(${JSON.stringify(options)}, {
get(target, name, receiver) {
if (name === 'clone') {
return structuredClone(target);
}
${!isSSR ? 'globalThis.astroAsset.referencedImages.add(target.fsPath);' : ''}
if (name === 'fsPath') {
return ${stringifiedFSPath};
}
${!isSSR ? `globalThis.astroAsset.referencedImages.add(${stringifiedFSPath});` : ''}
return target[name];
}
})
Expand Down
48 changes: 27 additions & 21 deletions packages/astro/src/assets/vite-plugin-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import { hashTransform, propsToFilename } from './utils/transformToPath.js';

const resolvedVirtualModuleId = '\0' + VIRTUAL_MODULE_ID;

const assetRegex = new RegExp(`\\.(${VALID_INPUT_FORMATS.join('|')})$`, 'i');
const assetRegex = new RegExp(`\\.(${VALID_INPUT_FORMATS.join('|')})`, 'i');
const assetRegexEnds = new RegExp(`\\.(${VALID_INPUT_FORMATS.join('|')})$`, 'i');

export default function assets({
settings,
Expand Down Expand Up @@ -81,7 +82,7 @@ export default function assets({
return;
}

globalThis.astroAsset.addStaticImage = (options, hashProperties) => {
globalThis.astroAsset.addStaticImage = (options, hashProperties, originalPath) => {
if (!globalThis.astroAsset.staticImages) {
globalThis.astroAsset.staticImages = new Map<
string,
Expand All @@ -97,11 +98,6 @@ export default function assets({
isESMImportedImage(options.src) ? options.src.src : options.src
).replace(settings.config.build.assetsPrefix || '', '');

// This, however, is the real original path, in `src` and all.
const originalSrcPath = isESMImportedImage(options.src)
? options.src.fsPath
: options.src;

const hash = hashTransform(
options,
settings.config.image.service.entrypoint,
Expand All @@ -120,7 +116,7 @@ export default function assets({

if (!transformsForPath) {
globalThis.astroAsset.staticImages.set(finalOriginalImagePath, {
originalSrcPath: originalSrcPath,
originalSrcPath: originalPath,
transforms: new Map(),
});
transformsForPath = globalThis.astroAsset.staticImages.get(finalOriginalImagePath)!;
Expand Down Expand Up @@ -178,15 +174,25 @@ export default function assets({
resolvedConfig = viteConfig;
},
async load(id, options) {
// If our import has any query params, we'll let Vite handle it
// See https://github.com/withastro/astro/issues/8333
if (id !== removeQueryString(id)) {
return;
}
if (assetRegex.test(id)) {
const meta = await emitESMImage(id, this.meta.watchMode, this.emitFile);
if (!globalThis.astroAsset.referencedImages)
globalThis.astroAsset.referencedImages = new Set();

if (id !== removeQueryString(id)) {
// If our import has any query params, we'll let Vite handle it, nonetheless we'll make sure to not delete it
// See https://github.com/withastro/astro/issues/8333
globalThis.astroAsset.referencedImages.add(removeQueryString(id));
return;
}

if (!meta) {
// If the requested ID doesn't end with a valid image extension, we'll let Vite handle it
if (!assetRegexEnds.test(id)) {
return;
}

const imageMetadata = await emitESMImage(id, this.meta.watchMode, this.emitFile);

if (!imageMetadata) {
throw new AstroError({
...AstroErrorData.ImageNotFound,
message: AstroErrorData.ImageNotFound.message(id),
Expand All @@ -197,13 +203,13 @@ export default function assets({
// Since you cannot use image optimization on the client anyway, it's safe to assume that if the user imported
// an image on the client, it should be present in the final build.
if (options?.ssr) {
return `export default ${getProxyCode(meta, isServerLikeOutput(settings.config))}`;
return `export default ${getProxyCode(
imageMetadata,
isServerLikeOutput(settings.config)
)}`;
} else {
if (!globalThis.astroAsset.referencedImages)
globalThis.astroAsset.referencedImages = new Set();

globalThis.astroAsset.referencedImages.add(meta.fsPath);
return `export default ${JSON.stringify(meta)}`;
globalThis.astroAsset.referencedImages.add(imageMetadata.fsPath);
return `export default ${JSON.stringify(imageMetadata)}`;
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/content/runtime-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export function createImage(pluginContext: PluginContext, entryFilePath: string)
return z.never();
}

return { ...metadata, ASTRO_ASSET: true };
return { ...metadata, ASTRO_ASSET: metadata.fsPath };
});
};
}
1 change: 1 addition & 0 deletions packages/astro/src/content/vite-plugin-content-imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,7 @@ function stringifyEntryData(data: Record<string, any>, isSSR: boolean): string {
// For Astro assets, add a proxy to track references
if (typeof value === 'object' && 'ASTRO_ASSET' in value) {
const { ASTRO_ASSET, ...asset } = value;
asset.fsPath = ASTRO_ASSET;
return getProxyCode(asset, isSSR);
}
});
Expand Down
17 changes: 15 additions & 2 deletions packages/astro/src/vite-plugin-astro/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface AstroPluginOptions {
export default function astro({ settings, logger }: AstroPluginOptions): vite.Plugin[] {
const { config } = settings;
let resolvedConfig: vite.ResolvedConfig;
let server: vite.ViteDevServer;

// Variables for determining if an id starts with /src...
const srcRootWeb = config.srcDir.pathname.slice(config.root.pathname.length - 1);
Expand All @@ -38,6 +39,9 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl
configResolved(_resolvedConfig) {
resolvedConfig = _resolvedConfig;
},
configureServer(_server) {
server = _server;
},
async load(id, opts) {
const parsedId = parseAstroRequest(id);
const query = parsedId.query;
Expand All @@ -46,9 +50,18 @@ export default function astro({ settings, logger }: AstroPluginOptions): vite.Pl
}
// For CSS / hoisted scripts, the main Astro module should already be cached
const filename = normalizePath(normalizeFilename(parsedId.filename, config.root));
const compileResult = getCachedCompileResult(config, filename);
let compileResult = getCachedCompileResult(config, filename);
if (!compileResult) {
return null;
// In dev, HMR could cause this compile result to be empty, try to load it first
if (server) {
await server.transformRequest('/@fs' + filename);
compileResult = getCachedCompileResult(config, filename);
}

// If there's really no compilation result, error
if (!compileResult) {
throw new Error('No cached compile result found for ' + id);
}
}

switch (query.type) {
Expand Down
11 changes: 11 additions & 0 deletions packages/astro/test/fixtures/core-image-deletion/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@test/core-image-deletion",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
},
"scripts": {
"dev": "astro dev"
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
import { Image } from "astro:assets";
import onlyOne from "../assets/onlyone.jpg";
import twoOfUs from "../assets/twoofus.jpg";
import twoFromURL from "../assets/url.jpg";
import twoFromURL_URL from "../assets/url.jpg?url";
---

<Image src={onlyOne} alt="Only one of me exists at the end of the build" />

<Image src={twoOfUs} alt="Two of us will exist, because I'm also used as a normal image" /></Image>
<img src={twoOfUs.src} alt="Two of us will exist, because I'm also used as a normal image" />

<Image src={twoFromURL} alt="Two of us will exist, because I'm also imported using ?url" /></Image>
<img src={twoFromURL_URL} alt="Two of us will exist, because I'm also used as a normal image" />
Loading

0 comments on commit a5fd580

Please sign in to comment.