-
Notifications
You must be signed in to change notification settings - Fork 394
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: react macro types #1620
fix: react macro types #1620
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
? { [P in K]?: T[K] } & Partial<Record<Exclude<keyof T, K>, never>> | ||
: never | ||
|
||
export type TransRenderCallbackOrComponent = |
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.
works the same as before but is a little simpler in terms of TS
had to name it so that it can be imported in macro typings.. maybe it could be renamed to something else so that it's not considered as stable public api..
const values = { ...props.values } | ||
const components = { ...props.components } |
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.
it is because "undefined" could be also spread to the object?
|
||
export function Trans(props: TransProps): React.ReactElement<any, any> | null { | ||
const { i18n, defaultComponent } = useLingui() | ||
const { render, component, id, message, formats } = props | ||
|
||
const values = { ...(props.values || {}) } | ||
const components = { ...(props.components || {}) } | ||
const values = { ...props.values } |
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.
spreading both null
and undefined
is valid so this works the same
id?: string | ||
comment?: string | ||
context?: string | ||
render?: (props: TransRenderProps) => ReactElement<any, any> | null | ||
i18n?: I18n |
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.
i18n
was added in #1308
I tested it and (1) doesn't work (2) isn't documented and right now it's easier to remove it
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.
Yes, i also spotted it there, and wrote an issue to clatify this #1433
size-limit report 📦
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1620 +/- ##
==========================================
+ Coverage 72.22% 75.40% +3.17%
==========================================
Files 72 77 +5
Lines 2459 1988 -471
Branches 708 517 -191
==========================================
- Hits 1776 1499 -277
+ Misses 536 375 -161
+ Partials 147 114 -33
☔ View full report in Codecov by Sentry. |
Description
improves macro TS typings
Types of changes
Fixes # (issue)
Checklist