feat: remove JSX namespace from element definition#335
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR removes the JSX namespace definition from the TypeScript definition file to avoid coupling with React's evolving type system. Instead of automatically declaring relative-time under JSX.IntrinsicElements, the PR adds documentation showing developers how to manually add React support if needed.
Key Changes:
- Removed automatic JSX namespace declaration from the TypeScript definition file
- Added React integration documentation with a code snippet for manual type declaration
- Cleaned up formatting throughout the README (whitespace, markdown tables, code blocks)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/relative-time-element-define.ts | Removed JSXBase type and JSX namespace declaration, keeping only the global HTMLElementTagNameMap definition |
| README.md | Added React usage section with manual JSX type declaration example; cleaned up formatting and whitespace throughout examples and tables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ##### time-zone (`string`) | ||
|
|
||
| The`time-zone` attribute allows you to specify the IANA time zone name (e.g., `America/New_York`, `Europe/London`) used for formatting the date and time. | ||
| The`time-zone` attribute allows you to specify the IANA time zone name (e.g., `America/New_York`, `Europe/London`) used for formatting the date and time. |
There was a problem hiding this comment.
Missing space after 'The' in 'Thetime-zone'.
| The`time-zone` attribute allows you to specify the IANA time zone name (e.g., `America/New_York`, `Europe/London`) used for formatting the date and time. | |
| The `time-zone` attribute allows you to specify the IANA time zone name (e.g., `America/New_York`, `Europe/London`) used for formatting the date and time. |
khiga8
left a comment
There was a problem hiding this comment.
Will this break existing instances of this element in dotcom, or will changes be made in dotcom to ensure it doesn't break things?
We use this element everywhere including in React so wanted to confirm!
|
@khiga8 should be good to go in dotcom! I believe Matt already updated instances in React to use RelativeTime from |
|
@joshblack amazing!!! |
Closes #333
With the changes to React's JSX namespace, this PR updates our
definemodule so that it no longer definesrelative-timeunderJSX.IntrinsicElements. Instead, we include a snippet in the README now that details how to use this alongside React. In the future we could also add a wrapper for this for React.This helps to avoid any future issues with changes to React's typescript types and means we don't have to specify any optional dependencies when using the
declare module 'react'syntax.Curious what you all think about this change 👀 Let me know if there would be a better way here!
In terms of versioning, since this would break existing usage of this element I believe we should treat this as a major change.