-
Notifications
You must be signed in to change notification settings - Fork 390
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: expression only choices / select options #1523
fix: expression only choices / select options #1523
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
@@ -352,8 +352,12 @@ export default class MacroJs { | |||
value = this.tokenizeTemplateLiteral(attrValue) | |||
} else if (this.types.isCallExpression(attrValue)) { | |||
value = this.tokenizeNode(attrValue) | |||
} else if (this.types.isStringLiteral(attrValue)) { | |||
value = attrValue.value | |||
} else if (this.types.isExpression(attrValue)) { |
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.
StringLiteral is also considered as Expression, so I moved StringLiteral check above. From ts, after all the checks, the types fall through are limited to babelTypes.RestElement | babelTypes.AssignmentPattern | babelTypes.ArrayPattern | babelTypes.ObjectPattern
, not sure exactly how we want to handle them, so I kept the previous fallback case untouched.
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.
the unknown
conversion is just to make ts happy, please let me know if you have other suggestions to handle the remaining cases.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## next #1523 +/- ##
==========================================
+ Coverage 74.55% 75.70% +1.15%
==========================================
Files 76 79 +3
Lines 1965 1984 +19
Branches 515 518 +3
==========================================
+ Hits 1465 1502 +37
+ Misses 389 361 -28
- Partials 111 121 +10
... and 18 files with indirect coverage changes 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.
I think the same is true for JSX and components. Could you also check add that? And it also should be checked for SWC Plugin. I think it should work already but at least the same test cases should be added to have test suite aligned.
macroJsx's fallback is to |
swc-plugin change in lingui/swc-plugin#40 is there a guide how to run tests etc ? I have zero experience with rust before... I can read more about it but would be great if there is a command that I can just copy and paste to run the tests ;) edited: nvm, found that i can just install cargo and run |
Description
to fix #1522
Types of changes
Fixes # (issue) #1522
Checklist