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 3 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
65 changes: 39 additions & 26 deletions packages/mdx/lib/plugin/recma-jsx-rewrite.js
View file
Open in desktop
Expand Up @@ -72,8 +72,8 @@ export function recmaJsxRewrite(options = {}) {
let createErrorHelper
/** @type {Scope|null} */
let currentScope
/** @type {Map<string, string>} */
const invalidComponentNameToVariable = new Map()
/** @type {Map<string | number, string>} */
const idToInvalidComponentName = new Map()

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

let componentName = toJsxIdOrMemberExpression(['_components', id])
/** @type {(string | number)[]} */
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
let jsxIdExpression = ['_components', id]
if (isIdentifierName(id) === false) {
let invalidComponentName = invalidComponentNameToVariable.get(id)
let invalidComponentName = idToInvalidComponentName.get(id)
if (invalidComponentName === undefined) {
invalidComponentName = `_component${invalidComponentNameToVariable.size}`
invalidComponentNameToVariable.set(id, invalidComponentName)
invalidComponentName = `_component${idToInvalidComponentName.size}`
idToInvalidComponentName.set(id, invalidComponentName)
}

componentName = toJsxIdOrMemberExpression([invalidComponentName])
jsxIdExpression = [invalidComponentName]
}

node.openingElement.name = componentName
node.openingElement.name =
toJsxIdOrMemberExpression(jsxIdExpression)

if (node.closingElement) {
node.closingElement.name = componentName
node.closingElement.name =
toJsxIdOrMemberExpression(jsxIdExpression)
}
}
}
Expand Down Expand Up @@ -240,17 +243,17 @@ export function recmaJsxRewrite(options = {}) {
let name

for (name of scope.tags) {
if (isIdentifierName(name)) {
defaults.push({
type: 'Property',
kind: 'init',
key: {type: 'Identifier', name},
value: {type: 'Literal', value: name},
method: false,
shorthand: false,
computed: false
})
}
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
})
}

actual.push(...scope.components)
Expand All @@ -269,7 +272,7 @@ export function recmaJsxRewrite(options = {}) {
if (
defaults.length > 0 ||
actual.length > 0 ||
invalidComponentNameToVariable.size > 0
idToInvalidComponentName.size > 0
) {
if (providerImportSource) {
importProvider = true
Expand Down Expand Up @@ -356,13 +359,23 @@ export function recmaJsxRewrite(options = {}) {
}

for (const [
invalidComponentName,
variable
] of invalidComponentNameToVariable) {
_componentsId,
_componentName
] of idToInvalidComponentName) {
Copy link
Member

Choose a reason for hiding this comment

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

I still kinda like name, value better, but I’m not blocking on this.
I feel a bit more strongly about not using _ to start variable names, that is often used with variables (parameters) to mark them as explicitly not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Understood, removed those _s! I also tried using name and value, but noticed it leads to {value: name} and {name: value} in the sections where they're used. It... kinda made my head spin, so I chose [id, componentName] instead 😆

// For component IDs that render invalid JSX (ex. <_components.custom-element>),
// Generate a separate variable to index into `_components`
// i.e. `const _component0 = _components['custom-element']`
// `return <_component0>...`
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
declarations.push({
type: 'VariableDeclarator',
id: {type: 'Identifier', name: variable},
init: {type: 'Literal', value: invalidComponentName}
id: {type: 'Identifier', name: _componentName},
init: {
type: 'MemberExpression',
object: {type: 'Identifier', name: '_components'},
property: {type: 'Literal', value: _componentsId},
computed: true,
optional: false
}
})
}

Expand Down
6 changes: 4 additions & 2 deletions packages/mdx/test/compile.js
View file
Open in desktop
Expand Up @@ -858,11 +858,13 @@ test('jsx', async () => {
[
'/*@jsxRuntime automatic @jsxImportSource react*/',
'function _createMdxContent(props) {',
' const _components = props.components || ({}), _component0 = "a-b";',
' const _components = Object.assign({',
' "a-b": "a-b"',
' }, props.components), _component0 = _components["a-b"];',
' return <>{<_component0></_component0>}</>;',
'}',
'function MDXContent(props = {}) {',
' const _component0 = "a-b", {wrapper: MDXLayout} = props.components || ({});',
' const _component0 = _components["a-b"], {wrapper: MDXLayout} = props.components || ({});',
bholmesdev marked this conversation as resolved.
Show resolved Hide resolved
' return MDXLayout ? <MDXLayout {...props}><_createMdxContent {...props} /></MDXLayout> : _createMdxContent(props);',
'}',
'export default MDXContent;',
Expand Down