Skip to content

Commit

Permalink
fix(markdoc & mdx): Proxy crimes (#10278)
Browse files Browse the repository at this point in the history
* fix(markdoc & mdx): Proxy cimes

* chore: changeset
  • Loading branch information
Princesseuh authored Mar 1, 2024
1 parent 87a3d51 commit a548a3a
Show file tree
Hide file tree
Showing 16 changed files with 108 additions and 5 deletions.
6 changes: 6 additions & 0 deletions .changeset/giant-spies-type.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@astrojs/markdoc": patch
"astro": patch
---

Fixes original images sometimes being kept / deleted when they shouldn't in both MDX and Markdoc
1 change: 1 addition & 0 deletions packages/astro/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
"dev": "astro-scripts dev --copy-wasm --prebuild \"src/runtime/server/astro-island.ts\" --prebuild \"src/runtime/client/{idle,load,media,only,visible}.ts\" \"src/**/*.{ts,js}\"",
"postbuild": "astro-scripts copy \"src/**/*.astro\" && astro-scripts copy \"src/**/*.wasm\"",
"test": "pnpm run test:node",
"test:match": "pnpm run test:node --match",
"test:e2e": "playwright test",
"test:e2e:match": "playwright test -g",
"test:node": "astro-scripts test \"test/**/*.test.js\""
Expand Down
8 changes: 7 additions & 1 deletion packages/astro/src/assets/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,13 @@ export async function generateImagesForPath(
!globalThis.astroAsset.referencedImages?.has(transformsAndPath.originalSrcPath)
) {
try {
await fs.promises.unlink(getFullImagePath(originalFilePath, env));
if (transformsAndPath.originalSrcPath) {
env.logger.debug(
'assets',
`Deleting ${originalFilePath} as it's not referenced outside of image processing.`
);
await fs.promises.unlink(getFullImagePath(originalFilePath, env));
}
} catch (e) {
/* No-op, it's okay if we fail to delete one of the file, we're not too picky. */
}
Expand Down
6 changes: 5 additions & 1 deletion packages/astro/src/assets/utils/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ export function getProxyCode(options: ImageMetadata, isSSR: boolean): string {
if (name === 'fsPath') {
return ${stringifiedFSPath};
}
${!isSSR ? `globalThis.astroAsset.referencedImages.add(${stringifiedFSPath});` : ''}
${
!isSSR
? `if (target[name] !== undefined) globalThis.astroAsset.referencedImages.add(${stringifiedFSPath});`
: ''
}
return target[name];
}
})
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/logger/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export type LoggerLabel =
| 'preferences'
| 'redirects'
| 'toolbar'
| 'assets'
// SKIP_FORMAT: A special label that tells the logger not to apply any formatting.
// Useful for messages that are already formatted, like the server start message.
| 'SKIP_FORMAT';
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import mdx from '@astrojs/mdx';
import markdoc from '@astrojs/markdoc';
import { defineConfig } from 'astro/config';

export default defineConfig({
integrations: [mdx(), markdoc()],
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
"astro": "workspace:*",
"@astrojs/mdx": "workspace:*",
"@astrojs/markdoc": "workspace:*"
},
"scripts": {
"dev": "astro dev"
Expand Down
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,7 @@
---
title: "Markdoc"
---

There is an image below

![A penguin](../../assets/markdocStillExists.jpg)
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
title: "MDX"
---

There is an image below

![A penguin](../../assets/mdxDontExist.jpg)
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { defineCollection, z } from 'astro:content';

const blog = defineCollection({
type: 'content',
schema: z.object({
title: z.string(),
})
});

export const collections = {
blog: blog,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
import type { GetStaticPaths } from "astro";
import { getCollection } from "astro:content";
export const getStaticPaths = (async () => {
const blog = await getCollection("blog");
return blog.map((post) => ({
params: {
slug: post.slug,
},
props: {
post
}
}));
}) satisfies GetStaticPaths;
const { post } = Astro.props;
const { Content } = await post.render();
---

<Content />
12 changes: 11 additions & 1 deletion packages/astro/test/image-deletion.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { before, describe, it } from 'node:test';
import { testImageService } from './test-image-service.js';
import { loadFixture } from './test-utils.js';

describe('astro:assets - delete images that are unused', () => {
describe('astro:assets - delete images that are unused zzz', () => {
/** @type {import('./test-utils.js').Fixture} */
let fixture;

Expand Down Expand Up @@ -33,5 +33,15 @@ describe('astro:assets - delete images that are unused', () => {
const imagesUsedElsewhere = await fixture.glob('_astro/url.*.*');
assert.equal(imagesUsedElsewhere.length, 2);
});

it('should delete MDX images only used for optimization', async () => {
const imagesOnlyOptimized = await fixture.glob('_astro/mdxDontExist.*.*');
assert.equal(imagesOnlyOptimized.length, 1);
});

it('should always keep Markdoc images', async () => {
const imagesUsedElsewhere = await fixture.glob('_astro/markdocStillExists.*.*');
assert.equal(imagesUsedElsewhere.length, 2);
});
});
});
14 changes: 13 additions & 1 deletion packages/integrations/markdoc/src/content-entry-type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,19 @@ async function emitOptimizedImages(
ctx.pluginContext.meta.watchMode,
ctx.pluginContext.emitFile
);
node.attributes[attributeName] = src;

const fsPath = resolved.id;

if (src) {
// We cannot track images in Markdoc, Markdoc rendering always strips out the proxy. As such, we'll always
// assume that the image is referenced elsewhere, to be on safer side.
if (ctx.astroConfig.output === 'static') {
if (globalThis.astroAsset.referencedImages)
globalThis.astroAsset.referencedImages.add(fsPath);
}

node.attributes[attributeName] = { ...src, fsPath };
}
} else {
throw new MarkdocError({
message: `Could not resolve image ${JSON.stringify(
Expand Down
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a548a3a

Please sign in to comment.