-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
feat(mdx-loader): the table-of-contents should display toc/headings of imported MDX partials #9684
Changes from 2 commits
c6b4b33
aa9caae
7f85855
be539db
f386394
51f5d41
1fdd6af
0b6cc68
bfea370
604febd
758e39b
c7f5c14
3121dce
9fec883
baeb5bd
04636de
1e00363
5d68e76
084f029
2c0df85
763d208
418d95e
9e4f8aa
25c6a70
925661d
9d3c544
65592a6
0348aa4
d070950
728d705
86f7275
f193b12
239550b
9b258e7
3d43631
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,15 +7,15 @@ | |
|
||
import {parse, type ParserOptions} from '@babel/parser'; | ||
import traverse from '@babel/traverse'; | ||
import stringifyObject from 'stringify-object'; | ||
import {toValue} from '../utils'; | ||
import {constructArrayString, toValue} from '../utils'; | ||
import type {Identifier} from '@babel/types'; | ||
import type {Node, Parent} from 'unist'; | ||
import type {Heading, Literal} from 'mdast'; | ||
// @ts-expect-error: TODO see https://github.com/microsoft/TypeScript/issues/49721 | ||
import type {Transformer} from 'unified'; | ||
import type { | ||
MdxjsEsm, | ||
MdxJsxFlowElement, | ||
// @ts-expect-error: TODO see https://github.com/microsoft/TypeScript/issues/49721 | ||
} from 'mdast-util-mdx'; | ||
|
||
|
@@ -93,21 +93,75 @@ const plugin: Plugin = function plugin( | |
const {toString} = await import('mdast-util-to-string'); | ||
const {visit} = await import('unist-util-visit'); | ||
|
||
const headings: TOCItem[] = []; | ||
const partialComponentToHeadingsName: {[key: string]: string} = | ||
Object.create(null); | ||
anatolykopyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
visit(root, 'heading', (child: Heading) => { | ||
const value = toString(child); | ||
// TOCItem or string already with the spread operator | ||
const headings: (TOCItem | string)[] = []; | ||
|
||
// depth:1 headings are titles and not included in the TOC | ||
if (!value || child.depth < 2) { | ||
return; | ||
visit(root, ['heading', 'mdxjsEsm', 'mdxJsxFlowElement'], (child) => { | ||
if (child.type === 'heading') { | ||
const headingNode = child as Heading; | ||
const value = toString(headingNode); | ||
|
||
// depth:1 headings are titles and not included in the TOC | ||
if (!value || headingNode.depth < 2) { | ||
return; | ||
} | ||
|
||
headings.push({ | ||
value: toValue(headingNode, toString), | ||
id: headingNode.data!.id!, | ||
level: headingNode.depth, | ||
}); | ||
} | ||
|
||
headings.push({ | ||
value: toValue(child, toString), | ||
id: child.data!.id!, | ||
level: child.depth, | ||
}); | ||
if (child.type === 'mdxjsEsm') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know the original code was already low-level, but I'd appreciate if we could make it more readable and high-level. I mean the intent of this algorithm should be clear without spending time trying to understand the implementation details. Currently it's a bit hard to see the big picture of it. I would extract things in smaller functions for example. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved the visitor for each type of node into its own function. This is a bit more readable now, but I can try to do some further reorganisation if needed. |
||
const importNode = child as MdxjsEsm; | ||
if (!importNode.data?.estree) { | ||
return; | ||
} | ||
|
||
for (const importDeclaration of importNode.data.estree.body) { | ||
if (importDeclaration.type !== 'ImportDeclaration') { | ||
continue; | ||
} | ||
const importPath = importDeclaration.source.value as string; | ||
const isMdxImport = /\.mdx?$/.test(importPath); | ||
if (!isMdxImport) { | ||
continue; | ||
} | ||
|
||
const componentName = importDeclaration.specifiers.find( | ||
(o: any) => o.type === 'ImportDefaultSpecifier', | ||
)?.local.name; | ||
|
||
if (!componentName) { | ||
continue; | ||
} | ||
const {length} = Object.keys(partialComponentToHeadingsName); | ||
const exportAsName = `${name}${length}`; | ||
anatolykopyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
partialComponentToHeadingsName[componentName] = exportAsName; | ||
|
||
importDeclaration.specifiers.push({ | ||
type: 'ImportSpecifier', | ||
imported: {type: 'Identifier', name}, | ||
local: {type: 'Identifier', name: exportAsName}, | ||
}); | ||
} | ||
} | ||
|
||
if (child.type === 'mdxJsxFlowElement') { | ||
const node = child as MdxJsxFlowElement; | ||
const nodeName = node.name; | ||
if (!nodeName) { | ||
return; | ||
} | ||
const headingsName = partialComponentToHeadingsName[nodeName]; | ||
if (headingsName) { | ||
headings.push(`...${headingsName}`); | ||
slorber marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
}); | ||
|
||
const {children} = root as Parent; | ||
|
@@ -124,9 +178,21 @@ export default plugin; | |
async function createExportNode(name: string, object: any): Promise<MdxjsEsm> { | ||
anatolykopyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const {valueToEstree} = await import('estree-util-value-to-estree'); | ||
|
||
const tocObject = object.map((heading: TOCItem | string) => { | ||
if (typeof heading === 'string') { | ||
const argumentName = heading.replace('...', ''); | ||
anatolykopyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return { | ||
type: 'SpreadElement', | ||
argument: {type: 'Identifier', name: argumentName}, | ||
}; | ||
} | ||
|
||
return valueToEstree(heading); | ||
}); | ||
|
||
return { | ||
type: 'mdxjsEsm', | ||
value: `export const ${name} = ${stringifyObject(object)}`, | ||
value: `export const ${name} = ${constructArrayString(object)}`, | ||
slorber marked this conversation as resolved.
Show resolved
Hide resolved
|
||
data: { | ||
estree: { | ||
type: 'Program', | ||
|
@@ -142,7 +208,10 @@ async function createExportNode(name: string, object: any): Promise<MdxjsEsm> { | |
type: 'Identifier', | ||
name, | ||
}, | ||
init: valueToEstree(object), | ||
init: { | ||
type: 'ArrayExpression', | ||
elements: tocObject, | ||
}, | ||
}, | ||
], | ||
kind: 'const', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
### Another page partial content | ||
|
||
This is text coming from another page partial | ||
|
||
#### Foo | ||
|
||
Level 4 headings don't belong in ToC |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
import PagePartial from './_pagePartial.mdx'; | ||
anatolykopyl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
import AnotherPagePartial from './_anotherPagePartial.mdx'; | ||
|
||
# Partials tests | ||
|
||
This page consists of multiple files imported into one. Notice how the table of contents works even for imported headings. | ||
|
||
## Imported content | ||
|
||
<PagePartial /> | ||
|
||
<AnotherPagePartial /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toc0 is not imported here, is this expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like we're going down the road of only modifying the AST and not the actual js, so this is expected.