-
Notifications
You must be signed in to change notification settings - Fork 393
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: fix type mismatch between formatter and conf/CatalogFormatter #1546
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -61,6 +62,12 @@ expectAssignable<LinguiConfig>({ | |||
], | |||
}) | |||
|
|||
// formatter | |||
expectAssignable<LinguiConfig>({ |
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.
not sure if this is the right way to add the test, as apparently the same test case also passed on next branch...
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.
this test should be in format-po
package.
@@ -62,12 +62,12 @@ export type CatalogFormatter = { | |||
templateExtension?: string | |||
parse( | |||
content: string, | |||
ctx: { locale: string | null; sourceLocale: string; filename: string } | |||
ctx: { locale: string; sourceLocale: string; filename: string } |
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.
from the call site, it looks like the locale
is always present, while existing
may not.
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.
locale may not be present in case if you generating a template.
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.
please return | null
back
size-limit report 📦
|
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## next #1546 +/- ##
=======================================
Coverage 75.34% 75.34%
=======================================
Files 78 78
Lines 1971 1971
Branches 515 515
=======================================
Hits 1485 1485
Misses 375 375
Partials 111 111
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
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.
this is incorrect fix, let me do this properly in another PR.
@@ -61,6 +62,12 @@ expectAssignable<LinguiConfig>({ | |||
], | |||
}) | |||
|
|||
// formatter | |||
expectAssignable<LinguiConfig>({ |
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.
this test should be in format-po
package.
@@ -62,12 +62,12 @@ export type CatalogFormatter = { | |||
templateExtension?: string | |||
parse( | |||
content: string, | |||
ctx: { locale: string | null; sourceLocale: string; filename: string } | |||
ctx: { locale: string; sourceLocale: string; filename: string } |
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.
please return | null
back
Suppressed by #1547 |
Description
fix type mismatch:
Types of changes
Fixes # (issue)
Checklist