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.

Sign up for GitHub

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

Jump to bottom

feat(mdx-loader): wrap mdx content title (# Title) in <header> for concistency #10335

Merged
merged 5 commits into from
Jul 25, 2024
Merged

feat(mdx-loader): wrap mdx content title (# Title) in <header> for concistency #10335

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16e6944
feat: wrap mdx title h1 in header
OzakIOne Jul 23, 2024
0dabd11
Fix tests
slorber Jul 25, 2024
76da744
disable /showcase from argos
slorber Jul 25, 2024
d891a87
add useful comment
slorber Jul 25, 2024
8fae018
empty
slorber Jul 25, 2024
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
21 changes: 15 additions & 6 deletions packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,25 @@
describe('contentTitle remark plugin', () => {
describe('extracts data.contentTitle', () => {
it('extracts h1 heading', async () => {
const result = await process(`
const result = await process(
`
# contentTitle 1

## Heading Two {#custom-heading-two}

# contentTitle 2

some **markdown** *content*
`);
`,
{removeContentTitle: true},
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some test needed removeContentTitle: true throw Cannot handle unknown node 'mdxJsxFlowElement' and test returns content unmodified still fails thrown: "Exceeded timeout of 15000 ms for a test.

Why would that test now need removeContentTitle: true. I tested locally and if it now timeouts, there must be a very good reason that we should find out.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first I couldn't really understand, and thought the timeout was coming from the visitor, but it turns out our test setup is not good and does not permit to stringify back to Markdown elements such as mdxJsxFlowElement

Going to fix it


expect(result.data.contentTitle).toBe('contentTitle 1');
});

it('extracts h1 heading alt syntax', async () => {
const result = await process(`
const result = await process(
`
contentTitle alt
===

Expand All @@ -44,7 +48,9 @@
# contentTitle 2

some **markdown** *content*
`);
`,
{removeContentTitle: true},
);

expect(result.data.contentTitle).toBe('contentTitle alt');
});
Expand Down Expand Up @@ -87,18 +93,21 @@
});

it('is able to decently serialize Markdown syntax', async () => {
const result = await process(`
const result = await process(
`
# some **markdown** \`content\` _italic_

some **markdown** *content*
`);
`,
{removeContentTitle: true},
);

expect(result.data.contentTitle).toBe('some markdown content italic');
});
});

describe('returns appropriate content', () => {
it('returns content unmodified', async () => {

Check failure on line 110 in packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts

View workflow job for this annotation

GitHub Actions / Tests (18.0)

contentTitle remark plugin › returns appropriate content › returns content unmodified 

thrown: "Exceeded timeout of 15000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

    at it (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:110:5)
    at describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:109:3)
    at Object.describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:19:1)

Check failure on line 110 in packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts

View workflow job for this annotation

GitHub Actions / Tests (20)

contentTitle remark plugin › returns appropriate content › returns content unmodified 

thrown: "Exceeded timeout of 15000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

    at it (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:110:5)
    at describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:109:3)
    at Object.describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:19:1)

Check failure on line 110 in packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts

View workflow job for this annotation

GitHub Actions / Tests (22.4)

contentTitle remark plugin › returns appropriate content › returns content unmodified 

thrown: "Exceeded timeout of 15000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

    at it (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:110:5)
    at describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:109:3)
    at Object.describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:19:1)

Check failure on line 110 in packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts

View workflow job for this annotation

GitHub Actions / Windows Tests (22.4)

contentTitle remark plugin › returns appropriate content › returns content unmodified 

thrown: "Exceeded timeout of 15000 ms for a test.
Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

    at it (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:110:5)
    at describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:109:3)
    at Object.describe (packages/docusaurus-mdx-loader/src/remark/contentTitle/__tests__/index.test.ts:19:1)
const content = `
# contentTitle 1

Expand Down
21 changes: 20 additions & 1 deletion packages/docusaurus-mdx-loader/src/remark/contentTitle/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// @ts-expect-error: TODO see https://github.com/microsoft/TypeScript/issues/49721
import type {Transformer} from 'unified';
import type {Heading} from 'mdast';
import type {Heading, Parent, RootContent} from 'mdast';

// TODO as of April 2023, no way to import/re-export this ESM type easily :/
// TODO upgrade to TS 5.3
Expand All @@ -19,6 +19,21 @@ interface PluginOptions {
removeContentTitle?: boolean;
}

function wrapHeadingInJsxHeader(
headingNode: Heading,
parent: Parent,
index: number | undefined,
) {
const article: RootContent = {
type: 'mdxJsxFlowElement',
name: 'header',
attributes: [],
children: [headingNode],
};
// @ts-expect-error: TODO how to fix?
parent.children[index] = article;
}

/**
* A remark plugin to extract the h1 heading found in Markdown files
* This is exposed as "data.contentTitle" to the processed vfile
Expand All @@ -37,11 +52,15 @@ const plugin: Plugin = function plugin(
visit(root, ['heading', 'thematicBreak'], (node, index, parent) => {
if (node.type === 'heading') {
const headingNode = node as Heading;
// console.log('headingNode:', headingNode);

if (headingNode.depth === 1) {
vfile.data.contentTitle = toString(headingNode);
if (removeContentTitle) {
// @ts-expect-error: TODO how to fix?
parent!.children.splice(index, 1);
} else {
wrapHeadingInJsxHeader(headingNode, parent, index);
}
return EXIT; // We only handle the very first heading
}
Expand Down
Loading