-
Notifications
You must be signed in to change notification settings - Fork 186
feat: remove JSX namespace from element definition #335
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
base: main
Are you sure you want to change the base?
Conversation
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.
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. |
Copilot
AI
Nov 20, 2025
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.