Conversation
- take 1, theres a bit more to do
maximilianfalco
requested review from
Jadenzzz,
eaglethrost,
kevinports and
rafegoldberg
Jadenzzz
left a comment
Jadenzzz
left a comment
There was a problem hiding this comment.
LGTM! looks much more organized
| /** Recursively searches a hast tree and returns the first element matching the given tag name. */ | ||
| export function findElementByTagName(node: Root | RootContent, tagName: string): Element | null { | ||
| if ('type' in node && node.type === 'element' && 'tagName' in node && node.tagName === tagName) { | ||
| return node; | ||
| } | ||
|
|
||
| if ('children' in node && Array.isArray(node.children)) { | ||
| return node.children.reduce<Element | null>((found, child) => { | ||
| if (found) return found; | ||
| return findElementByTagName(child, tagName); | ||
| }, null); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /** Recursively searches a hast tree and returns all elements matching the given tag name. */ | ||
| export function findElementsByTagName(node: Root | RootContent, tagName: string): Element[] { | ||
| const results: Element[] = []; | ||
|
|
||
| if ('type' in node && node.type === 'element' && 'tagName' in node && node.tagName === tagName) { | ||
| results.push(node); | ||
| } | ||
|
|
||
| if ('children' in node && Array.isArray(node.children)) { | ||
| node.children.forEach(child => { | ||
| results.push(...findElementsByTagName(child, tagName)); | ||
| }); | ||
| } | ||
|
|
||
| return results; | ||
| } |
There was a problem hiding this comment.
Ty for moving these into shared helpers, cleans up so much repetetive code!
__tests__/helpers.ts
Outdated
| default: () => null, | ||
| Toc: null, | ||
| toc: [], | ||
| } as unknown as RMDXModule; |
There was a problem hiding this comment.
Minor detail but is it possible to avoid casting with an assertion here?
| } as unknown as RMDXModule; | |
| } satisfies RMDXModule; |
There was a problem hiding this comment.
okay yea this makes sense but requires us to change this
default: () => null as unknown as React.JSX.Element,
basically adding the as unknown as above it
There was a problem hiding this comment.
Appreciate normalizing the casing here. But are the mdxish- prefixes now necessary for files inside the mdxish/ directory? So files now follow a <feature>.test.ts pattern but some have the prefix.
There was a problem hiding this comment.
i think it should be fine as is since they are already in an mdxish subdir, do you reckon we need the prefix?
Co-authored-by: Kevin Ports <kevin.ports@gmail.com>
…rkdown into falco/mdxish-cleanup
4 participants
🧰 Changes
DRY-ing and cleaning up tests files for
mdxishNote
This is a step 1 of a bigger attempt at cleaning up everything in the test suites.
New test file structure
🧬 QA & Testing