-
-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[Typescript] Enum types #18531
[Typescript] Enum types #18531
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.
Nice work.
I would have given a bit more details in the help:
|enumType|Specify the enum type which should be used in the client code.|<dl><dt>**stringUnion**</dt><dd>Union of literal string types</dd><dt>**enum**</dt><dd>Typescript's [string enums](https://www.typescriptlang.org/docs/handbook/enums.html#string-enums)</dd></dl>|stringUnion|
But I realized, that doing this in the java-generated cli help as well would be more difficult, so I think it's fine as-is.
Done |
@bodograumann |
Sorry, I don't have any elevated access here 😉 @ksvirkou-hubspot. Maybe @TiFu or @wing328 can. |
docs/generators/typescript.md
Outdated
@@ -24,6 +24,7 @@ These options may be applied as additional-properties (cli) or configOptions (pl | |||
|enumNameSuffix|Suffix that will be appended to all enum names.| |Enum| | |||
|enumPropertyNaming|Naming convention for enum properties: 'camelCase', 'PascalCase', 'snake_case', 'UPPERCASE', and 'original'| |PascalCase| | |||
|enumPropertyNamingReplaceSpecialChar|Set to true to replace '-' and '+' symbols with 'minus_' and 'plus_' in enum of type string| |false| | |||
|enumType|Specify the enum type which should be used in the client code.|<dl><dt>**stringUnion**</dt><dd>Union of literal string types</dd><dt>**enum**</dt><dd>Typescript's [string enums](https://www.typescriptlang.org/docs/handbook/enums.html#string-enums)</dd></dl>|stringUnion| |
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 default to enum
for backwards compatibility
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 rational here was that a breaking change was (kind of accidentally) introduced in #14663.
So this pr fixes that.
Personally I would be ok with either solution.
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.
ah i see. well i guess i would still lean towards making enum
the default, since 7.0.0 was released a while ago
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've done it
Could you check it, please?
import { createConfig, http } from "wagmi"; export const config = createConfig({ |
Unfortunately this was only merged into the 8.0.x branch, @wing328 . |
#14569
Added
enumType
generator argument:stringUnion
-"available" | "pending" | "sold"
enum
-{ Available = 'available', Pending = 'pending', Sold = 'sold' }
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*
.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master
(upcoming 7.1.0 minor release - breaking changes with fallbacks),8.0.x
(breaking changes without fallbacks)