fix: incompatible with some react rules #489
Conversation
JounQin
added
🐛 type/bug
🦋 Changeset detectedLatest commit: 93766b3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## release-v2 #489 +/- ##
============================================
Coverage 100.00% 100.00%
============================================
Files 19 19
Lines 188 188
Branches 32 32
============================================
Hits 188 188 ☔ View full report in Codecov by Sentry. |
JounQin
requested review from
remcohaszing,
wooorm,
ChristianMurphy and
a team
| @@ -363,33 +381,52 @@ runAsWorker( | |||
| processed.add(node) | |||
|
|
|||
| function handleChildren( | |||
| node: BlockContent | MdastLiteral | PhrasingContent, | |||
| node: BlockContent | Literal | Paragraph | PhrasingContent, | |||
There was a problem hiding this comment.
Paragraphis inBlockContent.- Why is
Literalthere? That’s an abstract class that you most likely don’t need. What do you need? Perhaps you can replace all this withParentsfrommdast?
There was a problem hiding this comment.
For example Code in jsx:
<div>
```ts
export default 'hello world'
```
</div>There was a problem hiding this comment.
Code is in BlockContent already
Literal is an abstract interface. It doesn’t have a type. It’s useful for tools that introduce new node types.
Code is an actual node, with a type, that extends Literal.
There was a problem hiding this comment.
Yes, I'm using Literal for supporting all other possible Literal types.
There was a problem hiding this comment.
I just removed the unnecessary Code type, thanks for pointing it out.
There was a problem hiding this comment.
If you want to support arbitrary literals (and parents), use Literal and Parent?
But I don’t see it as useful. I recommend using the content types
There was a problem hiding this comment.
I'm fixing known issues for legacy v2, your suggestion may be applied to master branch instead?
There was a problem hiding this comment.
@wooorm Hi, are we good to merge this PR as-is for v2 only? I'll cherry pick the commit and adopt your review on v3 later.
| JSXElement['children'] | ||
| >((acc, child) => { | ||
| ? ( | ||
| node.children as Array<BlockContent | Literal | PhrasingContent> |
There was a problem hiding this comment.
If you just want every node, use Nodes from mdast.
There was a problem hiding this comment.
Unfortunately, this PR still targets release-v2 with @types/mdast v3.
Initial checklist
Description of changes
close #437
close #488
Note: This PR's target is
release-v2branch, so the release workflow should be run onrelease-v2branchcc @wooorm