Skip to content

Commit

Permalink
Fix memleak when project path contains '.md' (#5264)
Browse files Browse the repository at this point in the history
  • Loading branch information
VladCuciureanu committed Nov 2, 2022
1 parent 78145c7 commit 0d27c4a
Show file tree
Hide file tree
Showing 12 changed files with 94 additions and 19 deletions.
5 changes: 5 additions & 0 deletions .changeset/clever-files-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixed memleak caused by project dir names containing '.md' or '.mdx'
15 changes: 3 additions & 12 deletions packages/astro/src/core/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,10 @@ export function isURL(value: unknown): value is URL {
return Object.prototype.toString.call(value) === '[object URL]';
}
/** Check if a file is a markdown file based on its extension */
export function isMarkdownFile(
fileId: string,
option: { criteria: 'endsWith' | 'includes'; suffix?: string }
): boolean {
const _suffix = option.suffix ?? '';
if (option.criteria === 'endsWith') {
for (let markdownFileExtension of SUPPORTED_MARKDOWN_FILE_EXTENSIONS) {
if (fileId.endsWith(`${markdownFileExtension}${_suffix}`)) return true;
}
return false;
}
export function isMarkdownFile(fileId: string, option?: { suffix?: string }): boolean {
const _suffix = option?.suffix ?? '';
for (let markdownFileExtension of SUPPORTED_MARKDOWN_FILE_EXTENSIONS) {
if (fileId.includes(`${markdownFileExtension}${_suffix}`)) return true;
if (fileId.endsWith(`${markdownFileExtension}${_suffix}`)) return true;
}
return false;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-astro-postprocess/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export default function astro(_opts: AstroPluginOptions): Plugin {
name: 'astro:postprocess',
async transform(code, id) {
// Currently only supported in ".astro" and ".md" (or any alternative markdown file extension like `.markdown`) files
if (!id.endsWith('.astro') && !isMarkdownFile(id, { criteria: 'endsWith' })) {
if (!id.endsWith('.astro') && !isMarkdownFile(id)) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-jsx/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ export default function jsx({ settings, logging }: AstroPluginJSXOptions): Plugi

const { mode } = viteConfig;
// Shortcut: only use Astro renderer for MD and MDX files
if (id.includes('.mdx') || isMarkdownFile(id, { criteria: 'includes' })) {
if (id.endsWith('.mdx')) {
const { code: jsxCode } = await esbuild.transform(code, {
loader: getEsbuildLoader(path.extname(id)) as esbuild.Loader,
jsx: 'preserve',
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/src/vite-plugin-markdown-legacy/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,15 +90,15 @@ export default function markdown({ settings }: AstroPluginOptions): Plugin {
// Resolve any .md (or alternative extensions of markdown files like .markdown) files with the `?content` cache buster. This should only come from
// an already-resolved JS module wrapper. Needed to prevent infinite loops in Vite.
// Unclear if this is expected or if cache busting is just working around a Vite bug.
if (isMarkdownFile(id, { criteria: 'endsWith', suffix: MARKDOWN_CONTENT_FLAG })) {
if (isMarkdownFile(id, { suffix: MARKDOWN_CONTENT_FLAG })) {
const resolvedId = await this.resolve(id, importer, { skipSelf: true, ...options });
return resolvedId?.id.replace(MARKDOWN_CONTENT_FLAG, '');
}
// If the markdown file is imported from another file via ESM, resolve a JS representation
// that defers the markdown -> HTML rendering until it is needed. This is especially useful
// when fetching and then filtering many markdown files, like with import.meta.glob() or Astro.glob().
// Otherwise, resolve directly to the actual component.
if (isMarkdownFile(id, { criteria: 'endsWith' }) && !isRootImport(importer)) {
if (isMarkdownFile(id) && !isRootImport(importer)) {
const resolvedId = await this.resolve(id, importer, { skipSelf: true, ...options });
if (resolvedId) {
return resolvedId.id + MARKDOWN_IMPORT_FLAG;
Expand All @@ -111,7 +111,7 @@ export default function markdown({ settings }: AstroPluginOptions): Plugin {
// A markdown file has been imported via ESM!
// Return the file's JS representation, including all Markdown
// frontmatter and a deferred `import() of the compiled markdown content.
if (isMarkdownFile(id, { criteria: 'endsWith', suffix: MARKDOWN_IMPORT_FLAG })) {
if (isMarkdownFile(id, { suffix: MARKDOWN_IMPORT_FLAG })) {
const { fileId, fileUrl } = getFileInfo(id, config);

const source = await fs.promises.readFile(fileId, 'utf8');
Expand Down Expand Up @@ -151,7 +151,7 @@ export default function markdown({ settings }: AstroPluginOptions): Plugin {
// A markdown file is being rendered! This markdown file was either imported
// directly as a page in Vite, or it was a deferred render from a JS module.
// This returns the compiled markdown -> astro component that renders to HTML.
if (isMarkdownFile(id, { criteria: 'endsWith' })) {
if (isMarkdownFile(id)) {
const filename = normalizeFilename(id);
const source = await fs.promises.readFile(filename, 'utf8');
const renderOpts = config.markdown;
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/vite-plugin-markdown/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
// passing to the transform hook. This lets us get the truly raw value
// to escape "import.meta.env" ourselves.
async load(id) {
if (isMarkdownFile(id, { criteria: 'endsWith' })) {
if (isMarkdownFile(id)) {
const { fileId, fileUrl } = getFileInfo(id, settings.config);
const rawFile = await fs.promises.readFile(fileId, 'utf-8');
const raw = safeMatter(rawFile, id);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from 'astro/config';
import react from '@astrojs/react';

// https://astro.build/config
export default defineConfig({
integrations: [react()],
});
11 changes: 11 additions & 0 deletions packages/astro/test/fixtures/impostor-mdx-file/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@test/impostor-md-file",
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/react": "workspace:*",
"astro": "workspace:*",
"react": "^18.1.0",
"react-dom": "^18.1.0"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import {useState} from "react";

export default function Foo() {
const [bar, setBar] = useState("Baz");
return <div className="main-component">{bar}</div>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
import Foo from '../components/Foo.mdx.jsx';
---

<html>
<head>
<!-- Head Stuff -->
</head>
<body>
<Foo client:load />
</body>
</html>
31 changes: 31 additions & 0 deletions packages/astro/test/impostor-mdx-file.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect } from 'chai';
import { isWindows, loadFixture } from './test-utils.js';

let fixture;

describe('Impostor MDX File', () => {
before(async () => {
fixture = await loadFixture({
root: './fixtures/impostor-mdx-file/',
});
});
if (isWindows) return;

describe('dev', () => {
/** @type {import('./test-utils').Fixture} */
let devServer;

before(async () => {
devServer = await fixture.startDevServer();
});

after(async () => {
await devServer.stop();
});

it('does not crash when loading react component with .md or .mdx in name', async () => {
const result = await fixture.fetch('/').then((response) => response.text());
expect(result).to.contain('Baz');
});
});
});
12 changes: 12 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 0d27c4a

Please sign in to comment.