-
Notifications
You must be signed in to change notification settings - Fork 9
feat(jsx): add jsx
generator
#273
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
// Maps Node.js API stability indices (0-3) to UI component stability levels. | ||
export const STABILITY_LEVELS = [ | ||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
'danger', // (0) Deprecated | ||
'warning', // (1) Experimental | ||
'success', // (2) Stable | ||
'info', // (3) Legacy | ||
]; | ||
|
||
// Maps HTML tags to corresponding component names in @node-core/ui-components. | ||
export const TAG_TRANSFORMS = { | ||
pre: 'CodeBox', | ||
blockquote: 'Blockquote', | ||
}; | ||
|
||
// Maps API heading types to their CircularIcon props. | ||
export const ICON_SYMBOL_MAP = { | ||
event: { symbol: 'E', color: 'red' }, | ||
method: { symbol: 'M', color: 'red' }, | ||
property: { symbol: 'P', color: 'red' }, | ||
class: { symbol: 'C', color: 'red' }, | ||
module: { symbol: 'M', color: 'red' }, | ||
classMethod: { symbol: 'S', color: 'red' }, | ||
ctor: { symbol: 'C', color: 'red' }, | ||
}; | ||
|
||
// Maps API lifecycle change type identifiers to their human-readable labels. | ||
export const CHANGE_TYPES = { | ||
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. IMO, we should import @node-core/i18n and have these as translation strings. This can be done in a follow-up PR. To be clear, @nodejs/web-infra we shouldn't translate the API docs, but at the very least, navigational and non-main-content markers can be translated. |
||
added_in: 'Added in', | ||
deprecated_in: 'Deprecated in', | ||
removed_in: 'Removed in', | ||
introduced_in: 'Introduced in', | ||
}; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,68 @@ | ||||||
import { | ||||||
getCompatibleVersions, | ||||||
groupNodesByModule, | ||||||
} from '../../utils/generators.mjs'; | ||||||
import buildContent from './utils/buildContent.mjs'; | ||||||
import { getRemarkRecma } from '../../utils/remark.mjs'; | ||||||
import { buildSideBarDocPages } from './utils/buildBarProps.mjs'; | ||||||
|
||||||
/** | ||||||
* This generator generates a JSX AST from an input MDAST | ||||||
* | ||||||
* @typedef {Array<ApiDocMetadataEntry>} Input | ||||||
* | ||||||
* @type {GeneratorMetadata<Input, string>} | ||||||
*/ | ||||||
export default { | ||||||
name: 'jsx', | ||||||
version: '1.0.0', | ||||||
description: 'Generates JSX from the input AST', | ||||||
dependsOn: 'ast', | ||||||
|
||||||
/** | ||||||
* Generates a JSX AST | ||||||
* | ||||||
* @param {Input} entries | ||||||
* @param {Partial<GeneratorOptions>} options | ||||||
* @returns {Promise<string[]>} Array of generated content | ||||||
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.
Suggested change
|
||||||
*/ | ||||||
async generate(entries, { releases, version }) { | ||||||
const remarkRecma = getRemarkRecma(); | ||||||
const groupedModules = groupNodesByModule(entries); | ||||||
|
||||||
// Get sorted primary heading nodes | ||||||
const headNodes = entries | ||||||
.filter(node => node.heading.depth === 1) | ||||||
.sort((a, b) => a.heading.data.name.localeCompare(b.heading.data.name)); | ||||||
|
||||||
// Generate table of contents | ||||||
const docPages = buildSideBarDocPages(groupedModules, headNodes); | ||||||
|
||||||
// Process each head node and build content | ||||||
const results = await Promise.all( | ||||||
headNodes.map(entry => { | ||||||
const versions = getCompatibleVersions( | ||||||
entry.introduced_in, | ||||||
releases, | ||||||
true | ||||||
); | ||||||
|
||||||
const sideBarProps = { | ||||||
versions: versions.map(({ version }) => `v${version.version}`), | ||||||
currentVersion: `v${version.version}`, | ||||||
currentPage: `${entry.api}.html`, | ||||||
docPages, | ||||||
}; | ||||||
|
||||||
return buildContent( | ||||||
groupedModules.get(entry.api), | ||||||
entry, | ||||||
sideBarProps, | ||||||
remarkRecma | ||||||
); | ||||||
}) | ||||||
); | ||||||
|
||||||
return results; | ||||||
}, | ||||||
}; |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,71 @@ | ||||||
'use strict'; | ||||||
|
||||||
import { u as createTree } from 'unist-builder'; | ||||||
import { valueToEstree } from 'estree-util-value-to-estree'; | ||||||
|
||||||
/** | ||||||
* Creates an MDX JSX element with support for complex attribute values. | ||||||
* | ||||||
* @param {string} name - The name of the JSX element | ||||||
* @param {{ | ||||||
* inline?: boolean, | ||||||
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. Lines 11 to 13 should be indented 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. 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. Right, but I believe extra level, makes it easier to read:
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.
This can be changed, but it breaks our conventions 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. Noted, then because of the lint rule this got changed because when I started this repo I 100% did this sort of indentation xD I'm fine leaving as it is (feel free to resolve this thread)... but, IMO this makes readability better. 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. IMO if we are going to define an object as a property, I think it should be just single line. For better readability, everything else should be defined with |
||||||
* children?: string | import('unist').Node[], | ||||||
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.
Suggested change
|
||||||
* [key: string]: any | ||||||
* }} [options={}] - Options including type, children, and JSX attributes | ||||||
* @returns {import('unist').Node} The created MDX JSX element node | ||||||
*/ | ||||||
export const createJSXElement = ( | ||||||
name, | ||||||
{ inline = true, children = [], ...attributes } = {} | ||||||
) => { | ||||||
// Process children: convert string to text node or use array as is | ||||||
const processedChildren = | ||||||
typeof children === 'string' | ||||||
? [createTree('text', { value: children })] | ||||||
: (children ?? []); | ||||||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
// Create attribute nodes, handling complex objects and primitive values differently | ||||||
const attrs = Object.entries(attributes).map(([key, value]) => | ||||||
createAttributeNode(key, value) | ||||||
); | ||||||
|
||||||
// Create and return the appropriate JSX element type | ||||||
return createTree(inline ? 'mdxJsxTextElement' : 'mdxJsxFlowElement', { | ||||||
name, | ||||||
attributes: attrs, | ||||||
children: processedChildren, | ||||||
}); | ||||||
}; | ||||||
|
||||||
/** | ||||||
* Creates an MDX JSX attribute node from the input. | ||||||
* | ||||||
* @param {string} name - The attribute name | ||||||
* @param {any} value - The attribute value (can be any valid JS value) | ||||||
* @returns {import('unist').Node} The MDX JSX attribute node | ||||||
*/ | ||||||
function createAttributeNode(name, value) { | ||||||
// For objects and arrays, create expression nodes to preserve structure | ||||||
if (value !== null && typeof value === 'object') { | ||||||
return createTree('mdxJsxAttribute', { | ||||||
name, | ||||||
value: createTree('mdxJsxAttributeValueExpression', { | ||||||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
data: { | ||||||
estree: { | ||||||
type: 'Program', | ||||||
body: [ | ||||||
{ | ||||||
type: 'ExpressionStatement', | ||||||
expression: valueToEstree(value), | ||||||
}, | ||||||
], | ||||||
sourceType: 'module', | ||||||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
}, | ||||||
}, | ||||||
}), | ||||||
}); | ||||||
} | ||||||
|
||||||
// For primitives, use simple string conversion | ||||||
return createTree('mdxJsxAttribute', { name, value: String(value) }); | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
import readingTime from 'reading-time'; | ||
import { DOC_API_BLOB_EDIT_BASE_URL } from '../../../constants.mjs'; | ||
import { visit } from 'unist-util-visit'; | ||
|
||
/** | ||
* Builds sidebar navigation for API documentation pages | ||
* | ||
* @param {Map<string, Array<ApiDocMetadataEntry>>} groupedModules - Modules grouped by API | ||
* @param {Array<ApiDocMetadataEntry>} headNodes - Main entry nodes for each API | ||
*/ | ||
export const buildSideBarDocPages = (groupedModules, headNodes) => | ||
headNodes.map(node => { | ||
const moduleEntries = groupedModules.get(node.api); | ||
|
||
return { | ||
title: node.heading.data.name, | ||
doc: `${node.api}.html`, | ||
headings: moduleEntries | ||
.filter(entry => entry.heading?.data?.name && entry.heading.depth === 2) | ||
.map(entry => [entry.heading.data.name, `#${entry.heading.data.slug}`]), | ||
}; | ||
}); | ||
|
||
/** | ||
* Builds metadata for the sidebar and meta bar | ||
* | ||
* @param {ApiDocMetadataEntry} head - Main API metadata entry | ||
* @param {Array<ApiDocMetadataEntry>} entries - All API metadata entries | ||
*/ | ||
export const buildMetaBarProps = (head, entries) => { | ||
// Extract text content for reading time calculation | ||
let textContent = ''; | ||
avivkeller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
entries.forEach(entry => { | ||
visit(entry.content, ['text', 'code'], node => { | ||
textContent += node.value || ''; | ||
}); | ||
}); | ||
|
||
const headings = entries | ||
.filter(entry => entry.heading?.data?.name) | ||
.map(entry => ({ | ||
depth: entry.heading.depth, | ||
value: entry.heading.data.name, | ||
})); | ||
|
||
return { | ||
headings, | ||
addedIn: head.introduced_in || head.added_in || '', | ||
readingTime: readingTime(textContent).text, | ||
viewAs: [['JSON', `${head.api}.json`]], | ||
editThisPage: `${DOC_API_BLOB_EDIT_BASE_URL}${head.api}.md`, | ||
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. Don't we have a metadata property that is exactly this already? 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 don't think so, we use the same method of building it in 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. Might make sense to then make this as a property on the metadata itself 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 don't think it's needed. It's really just |
||
}; | ||
}; |
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.
If this is the intended behavior and the solution, we should move this dependency to node.js organization, before merging this pull-request. It will run on every postinstall, and it's huge security concern.
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.
We are still looking for solutions, see #236
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.
Agreed with @anonrig although I'd say this is a draft, but we def... need to find a perma solution.
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.
We should at the very least move this dep to @pkgjs. @avivkeller feel free to open an issue on /admin to transfer this + ownership.
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.
Shouldn't we first decide whether this is a perma solution? I'd argue that we shouldn't maintain a package that we aren't going to use-but I'm happy to transfer it to the org regardless
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.
Sure, but this PR cannot land till we decide that, or at least cannot land whilst git-deps is a dependency here 😅
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.
Per the vote in #236, it looks like nodejs/nodejs.org#7776 will resolve that, and once they are published, I'll switch it.
git-deps
allows me to test this PR until then.