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

Fix rewriting of components for custom elements #2101

Merged
merged 19 commits into from Aug 17, 2022
Merged

Fix rewriting of components for custom elements #2101

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
11b96d3
feat: add toValidIdentifierName util
bholmesdev Aug 11, 2022
1c86a45
feat: serialize custom element component keys
bholmesdev Aug 11, 2022
f1205fe
chore: update custom element test case
bholmesdev Aug 11, 2022
389b5a5
fix: update JSdoc reference
bholmesdev Aug 11, 2022
135aa2d
test: to-valid-identifier-name
bholmesdev Aug 11, 2022
0553e0c
Revert "test: to-valid-identifier-name"
bholmesdev Aug 12, 2022
8f5fbe0
Revert "fix: update JSdoc reference"
bholmesdev Aug 12, 2022
a3bd1de
Revert "chore: update custom element test case"
bholmesdev Aug 12, 2022
2ec33fa
Revert "feat: serialize custom element component keys"
bholmesdev Aug 12, 2022
5878dd8
Revert "feat: add toValidIdentifierName util"
bholmesdev Aug 12, 2022
451a76f
feat: inline variables for invalid comp names
bholmesdev Aug 12, 2022
e547aa8
test: update custom elements
bholmesdev Aug 12, 2022
7572cdc
fix: avoid duplicating jsx expr
bholmesdev Aug 15, 2022
604e6db
feat: change to "_componentX = _components['X']"
bholmesdev Aug 15, 2022
24fb34d
test: update output
bholmesdev Aug 15, 2022
d051f7c
nit: ...[] => Array<...>
bholmesdev Aug 15, 2022
f7aea66
nit: clarify inline docs
bholmesdev Aug 15, 2022
55414b4
refactor: [id, componentName]
bholmesdev Aug 15, 2022
a9d93b2
fix: only add _componentX to _createMdxContent
bholmesdev Aug 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 42 additions & 20 deletions packages/mdx/lib/plugin/recma-jsx-rewrite.js
View file
Open in desktop
Expand Up @@ -72,6 +72,8 @@ export function recmaJsxRewrite(options = {}) {
let createErrorHelper
/** @type {Scope|null} */
let currentScope
/** @type {Map<string, string>} */
const invalidComponentNameToVariable = new Map()

walk(tree, {
enter(_node) {
Expand Down Expand Up @@ -193,16 +195,21 @@ export function recmaJsxRewrite(options = {}) {
fnScope.tags.push(id)
}

node.openingElement.name = toJsxIdOrMemberExpression([
'_components',
id
])
let componentName = toJsxIdOrMemberExpression(['_components', id])
if (isIdentifierName(id) === false) {
let invalidComponentName = invalidComponentNameToVariable.get(id)
if (invalidComponentName === undefined) {
invalidComponentName = `_component${invalidComponentNameToVariable.size}`
invalidComponentNameToVariable.set(id, invalidComponentName)
}

componentName = toJsxIdOrMemberExpression([invalidComponentName])
}

node.openingElement.name = componentName

if (node.closingElement) {
node.closingElement.name = toJsxIdOrMemberExpression([
'_components',
id
])
node.closingElement.name = componentName
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -233,17 +240,17 @@ export function recmaJsxRewrite(options = {}) {
let name

for (name of scope.tags) {
defaults.push({
type: 'Property',
kind: 'init',
key: isIdentifierName(name)
? {type: 'Identifier', name}
: {type: 'Literal', value: name},
value: {type: 'Literal', value: name},
method: false,
shorthand: false,
computed: false
})
if (isIdentifierName(name)) {
defaults.push({
type: 'Property',
kind: 'init',
key: {type: 'Identifier', name},
value: {type: 'Literal', value: name},
method: false,
shorthand: false,
computed: false
})
}
}

actual.push(...scope.components)
Expand All @@ -259,7 +266,11 @@ export function recmaJsxRewrite(options = {}) {
/** @type {Array<Statement>} */
const statements = []

if (defaults.length > 0 || actual.length > 0) {
if (
defaults.length > 0 ||
actual.length > 0 ||
invalidComponentNameToVariable.size > 0
) {
if (providerImportSource) {
importProvider = true
parameters.push({
Expand Down Expand Up @@ -344,6 +355,17 @@ export function recmaJsxRewrite(options = {}) {
componentsInit = {type: 'Identifier', name: '_components'}
}

for (const [
invalidComponentName,
variable
] of invalidComponentNameToVariable) {
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
declarations.push({
type: 'VariableDeclarator',
id: {type: 'Identifier', name: variable},
init: {type: 'Literal', value: invalidComponentName}
})
}

if (componentsPattern) {
declarations.push({
type: 'VariableDeclarator',
Expand Down
8 changes: 3 additions & 5 deletions packages/mdx/test/compile.js
View file
Open in desktop
Expand Up @@ -858,13 +858,11 @@ test('jsx', async () => {
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' const _components = Object.assign({',
' "a-b": "a-b"',
' }, props.components);',
' return <>{<_components.a-b></_components.a-b>}</>;',
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
' const _components = props.components || ({}), _component0 = "a-b";',
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
' return <>{<_component0></_component0>}</>;',
'}',
'function MDXContent(props = {}) {',
' const {wrapper: MDXLayout} = props.components || ({});',
' const _component0 = "a-b", {wrapper: MDXLayout} = props.components || ({});',
Copy link
Member

Choose a reason for hiding this comment

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

Same here as above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I think I understand this prop a bit better now. Looks like I didn't implement this right! I'll rework 👍

Copy link
Member

Choose a reason for hiding this comment

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

Sweet, thanks!
You could also likely set it on that destructuring object, alongside wrapper: MDXLayout? You may need to set a computed field in the AST or so though 🤔: https://astexplorer.net/#/gist/702f0315bf76ddbaf45a0928b0b06e96/784539f882835483b4541f9eff8d81ec4a327733

' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
Expand Down