Handle 'true'/'false' stringified boolean options and better config error handling#520
Conversation
8a1acfd to
8949b37
Compare
Coerce 'true'/'false' into actual booleans (Fixes ember-codemods#223) Better error messages
|
|
||
| if (replaced) { | ||
| source = root.toSource({ | ||
| quote: userOptions.quotes ?? userOptions.quote, |
There was a problem hiding this comment.
quotes was never documented and was redundant with quote, so I removed it.
wycats
left a comment
There was a problem hiding this comment.
Looks good! I have a few (non-blocking) questions, mostly because I'm new to zod.
| @@ -0,0 +1,254 @@ | |||
| import { describe, expect, test } from '@jest/globals'; | |||
There was a problem hiding this comment.
I'm loving all these new tests. Great job!
transforms/helpers/options.ts
Outdated
| return typeof arg === 'string' ? arg.split(/\s*,\s*/) : arg; | ||
| }, z.array(z.string())), | ||
| inObjectLiterals: StringArraySchema.describe( | ||
| 'Allow-list for decorators currently applied to object literal properties that can be safely applied to class properties.' |
There was a problem hiding this comment.
I find this message pretty confusing. What exactly is it trying to say?
There was a problem hiding this comment.
Input:
Config:
Output:
Without the config, the @userAdded decorator would not transform at all because it's applied to a property and not a method.
There was a problem hiding this comment.
What about something like:
A list of decorators that are allowed in object literals.
When the codemod finds a field with one of these decorators,
it will be translated directly into a class field with the
same decorator. Including a decorator in this list means
that you believe that the decorator will work correctly
on a class field.
| @@ -1,49 +1,98 @@ | |||
| import { inspect } from 'node:util'; | |||
| import type { ZodError } from 'zod'; | |||
There was a problem hiding this comment.
I haven't tried ts-to-zod, but zod is already based on TS APIs and terminology, so it's super intuitive (to me) as-is.
| import type { ZodEffects, ZodTypeAny } from 'zod'; | ||
| import { z } from 'zod'; | ||
|
|
||
| const preprocessStringToBoolean = <I extends ZodTypeAny>( |
There was a problem hiding this comment.
This file seems like a good place to DRY up some of the union error message duplication.
There was a problem hiding this comment.
Based on your previous answer, it seems like you considered this but it wasn't worth it. Right?
There was a problem hiding this comment.
Correct. I tried it but didn't like the built-in error messages.
'true'/'false'into actual booleans (Fixes Passing boolean flag options not working correctly #223)Ideally, we'd use
yargsfor this, but doing so would involve changingcodemod-clito allow consumers to pass inyargsoptions here:https://github.com/rwjblue/codemod-cli/blob/4a126ec9cd0ff4a482a69aa7f872ec629e23ba66/src/options-support.js#L14