Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix memleak caused by directory names containing '.md' or '.mdx' #5264

Merged
merged 1 commit into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.