-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
refactor: matching jsx component logic in string fragments #1861
Closed
Closed
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import * as React from 'react'; | ||
|
||
export interface IdentityProps { | ||
[key: string]: React.ReactNode; | ||
} | ||
|
||
function Identity(props: React.PropsWithChildren<IdentityProps>) { | ||
const { children, ...restProps } = props; | ||
return ( | ||
<ul> | ||
{Object.entries(restProps).map(([key, value]) => ( | ||
<li key={key}>{value}</li> | ||
))} | ||
{children && <li>{children}</li>} | ||
</ul> | ||
); | ||
} | ||
|
||
export default Identity; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 17 additions & 0 deletions
17
src/loaders/markdown/transformer/__snapshots__/rehypeRaw.test.ts.snap
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html | ||
|
||
exports[`rehypeRaw > addCustomPropsToMatchingJSX > should match nested jsx components 1`] = ` | ||
"<Foo $tag-name=\\"Foo\\" props=\\"123\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\"></Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\">bar</Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\"><Foo $tag-name=\\"Foo\\" props=\\"123\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\"></Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\">bar</Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\"><Foo $tag-name=\\"Foo\\" props=\\"123\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\"></Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\">bar</Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\">bar</Foo></Foo></Foo>" | ||
`; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
import { __private__ } from './rehypeRaw'; | ||
|
||
const { addCustomPropsToMatchingJSX } = __private__; | ||
|
||
describe('rehypeRaw', () => { | ||
describe('addCustomPropsToMatchingJSX', () => { | ||
it('should return the original string when there are no jsx fragments to match', () => { | ||
const input = 'foo bar'; | ||
expect(addCustomPropsToMatchingJSX(input)).toBe(input); | ||
}); | ||
|
||
it('should match jsx fragments without children', () => { | ||
const input = '<Foo />'; | ||
expect(addCustomPropsToMatchingJSX(input)).toMatchInlineSnapshot( | ||
'"<Foo $tag-name=\\"Foo\\" />"', | ||
); | ||
}); | ||
|
||
it('should match jsx fragments that contain children', () => { | ||
const input = '<Foo>bar</Foo>'; | ||
expect(addCustomPropsToMatchingJSX(input)).toMatchInlineSnapshot( | ||
'"<Foo $tag-name=\\"Foo\\" >bar</Foo>"', | ||
); | ||
}); | ||
|
||
it('should match multiple jsx fragments', () => { | ||
const input = ` | ||
<Foo>bar</Foo> | ||
<Bar>foo</Bar> | ||
<Footer /> | ||
`.trim(); | ||
expect(addCustomPropsToMatchingJSX(input)).toMatchInlineSnapshot(` | ||
"<Foo $tag-name=\\"Foo\\" >bar</Foo> | ||
<Bar $tag-name=\\"Bar\\" >foo</Bar> | ||
<Footer $tag-name=\\"Footer\\" />" | ||
`); | ||
}); | ||
|
||
it('should match jsx fragments containing props', () => { | ||
const input = ` | ||
<Foo props="123" /> | ||
<Foo props="ReutersType<Props>" /> | ||
<Foo props="ReutersType<Props>"></Foo> | ||
<Foo props="ReutersType<Props>">bar</Foo> | ||
`.trim(); | ||
expect(addCustomPropsToMatchingJSX(input)).toMatchInlineSnapshot(` | ||
"<Foo $tag-name=\\"Foo\\" props=\\"123\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\" /> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\"></Foo> | ||
<Foo $tag-name=\\"Foo\\" props=\\"ReutersType<Props>\\">bar</Foo>" | ||
`); | ||
}); | ||
|
||
it('should match nested jsx components', () => { | ||
const createInput = (childrenStr: string) => | ||
` | ||
<Foo props="123" /> | ||
<Foo props="ReutersType<Props>" /> | ||
<Foo props="ReutersType<Props>"></Foo> | ||
<Foo props="ReutersType<Props>">bar</Foo> | ||
<Foo props="ReutersType<Props>">${childrenStr}</Foo> | ||
`.trim(); | ||
|
||
// nested | ||
const input = createInput(createInput(createInput('bar'))); | ||
expect(addCustomPropsToMatchingJSX(input)).toMatchSnapshot(); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
这个改动删掉的逻辑会导致驼峰 props 的丢失,比如
<Foo relativePath="xxx"></Foo>
会被转成<Foo relative-path="xxx"></Foo>
另外看了下关键改动点,应该就是正则的改动从匹配
<Foo
变成匹配<Foo xxx>
,虽然能解 issue 提到的 case 但仍然存在字符串属性值里的 JSX 被误伤的可能性,比如<Foo str="<Bar xxx></Bar>"
应该还是会误伤,只要还是用正则而不是 AST 误伤就不可避免建议还是按原逻辑替换,在最后一步遍历 properties 恢复 key 名的时候,多加一步恢复 value 的逻辑,整体流程是先误伤、再恢复,因为到恢复这一步的时候,hast-util-raw 已经通过 AST 解析把真正的标签处理过了,剩下的都是在 properties 里的字符串标签,正好是之前误伤的内容
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.
误伤后给 raw 处理得到的 properties 是有问题的。回复处理有点困难
我这边的解决思路还是用这个 pr, 然后继续跟进你说的 驼峰 props 丢失问题。先挂一下问题,对我来说有些挑战
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.
试了下 properties 的确已经乱了,我换了种思路解决,有空可以帮忙 review 下:#1872