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.

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

fix "th" and "td" type definitions based off native elements #714

Merged
merged 1 commit into from
Dec 1, 2022

Conversation

lucasassisrosa
Copy link
Contributor

@lucasassisrosa lucasassisrosa commented Nov 30, 2022

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

Addresses #713 . Specify "th" and "td" props based off html elements. ast-to-react.d.ts will look like:

export type TableDataCellProps = ComponentPropsWithoutRef<'td'> &
  ReactMarkdownProps & {
    style?: Record<string, unknown>
    isHeader: false
  }
export type TableHeaderCellProps = ComponentPropsWithoutRef<'th'> &
  ReactMarkdownProps & {
    style?: Record<string, unknown>
    isHeader: true
  }
(...)

export type TableDataCellComponent = ComponentType<TableDataCellProps>
export type TableHeaderCellComponent = ComponentType<TableHeaderCellProps>
(...)

export type SpecialComponents = {
  (...)
  td: TableDataCellComponent | ReactMarkdownNames
  th: TableHeaderCellComponent | ReactMarkdownNames
}

@github-actions github-actions bot added the 👋 phase/new Post is being triaged automatically label Nov 30, 2022
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@github-actions github-actions bot added 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Nov 30, 2022
@ChristianMurphy
Copy link
Member

Thanks @lucasassisrosa!
A few things:

  1. it would be good to get Missing props in "table" type definitions #713 (comment) answered/resolved before moving forward
  2. It would be good to add a test case covering this case

@wooorm wooorm merged commit 9b20440 into remarkjs:main Dec 1, 2022
@github-actions

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Dec 1, 2022

As it’s a clear typo in the types (td or th should be used instead of table), I think it’s fine to solve it. A test case improvement for

test('should pass `isHeader: true` to `th`s, `isHeader: false` to `td`s', () => {
const input = '| a |\n| - |\n| b |\n| c |'
const actual = asHtml(
<Markdown
children={input}
remarkPlugins={[gfm]}
components={{
th({node, isHeader, ...props}) {
assert.equal(isHeader, true)
return React.createElement('th', props)
},
td({node, isHeader, ...props}) {
assert.equal(isHeader, false)
return React.createElement('td', props)
}
}}
/>
)
const expected =
'<table><thead><tr><th>a</th></tr></thead><tbody><tr><td>b</td></tr><tr><td>c</td></tr></tbody></table>'
assert.equal(actual, expected)
})
would be useful though.

@wooorm wooorm added 🐛 type/bug This is a problem 💪 phase/solved Post is done labels Dec 1, 2022
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Dec 1, 2022
@wooorm
Copy link
Member

wooorm commented Dec 1, 2022

Released in 8.0.4!

@ChristianMurphy ChristianMurphy added the ☂️ area/types This affects typings label Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☂️ area/types This affects typings 💪 phase/solved Post is done 🐛 type/bug This is a problem
Development

Successfully merging this pull request may close these issues.

None yet

3 participants