-
-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat: default value functions #1139
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.
Definitely think this will be a positive change, have some suggestions
|
||
describe("works with objects", () => { | ||
// it("default array in string", () => { | ||
// const t = type({ bar: type("number[] = []") }) |
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 won't implement number[] = []
but it's a pretty idea nevertheless
ark/type/keywords/inference.ts
Outdated
@@ -434,6 +434,9 @@ export type Default<v = any> = { | |||
default?: { value: v } | |||
} | |||
|
|||
export type DefaultFor<t> = | |||
t extends Primitive ? t | (() => t) : (t & Primitive) | (() => t) |
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 need ifAnyOrUnknown
helper here, do you have one?
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.
currently it works "fine" (test pass) except allowing string&[] for []
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 this is what you want:
export type DefaultFor<t> =
| (() => t)
| (Primitive extends t ? Primitive
: t extends Primitive ? t
: never)
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 made
export type DefaultFor<t> =
| (unknown extends t ? Primitive
: 0 extends 1 & t ? Primitive
: t extends Primitive ? t
: never)
| (() => t)
year, yours seems better
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.
Pretty good though, I always find your solutions very impressive.
So much cases, so much cases |
Hmm, type({ a: 'number' }).default({ | }) doesn't have autocomplete, I don't like that |
ark/type/methods/base.ts
Outdated
default< | ||
const value extends this["inferIn"], | ||
const value extends Extract<this["inferIn"], Primitive>, | ||
r = applyConstraint<t, Default<value>> | ||
>( | ||
value: value | ||
): instantiateType<r, $> | ||
): NoInfer<instantiateType<r, $>> | ||
default< | ||
const value extends this["inferIn"], | ||
r = applyConstraint<t, Default<value>> | ||
>( | ||
value: () => value | ||
): NoInfer<instantiateType<r, $>> |
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.
should just use value: DefaultFor<value>
here, having overload with never
breaks autocompletion
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.
Seems good to use the same type util everywhere anyways
ark/schema/structure/optional.ts
Outdated
export const assertDefaultValueAssignability = ( | ||
node: BaseRoot, | ||
value: unknown, | ||
key = "" | ||
): unknown => { | ||
const out = node.in(value) | ||
if (!isPrimitive(value) && typeof value !== "function") |
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.
Let's just get rid of isPrimitive
and instead use hasDomain(value, "object")
(it works like the TS keyword so it handles this logic built in, and we use it throughout the repo)
ark/type/methods/base.ts
Outdated
default< | ||
const value extends this["inferIn"], | ||
const value extends Extract<this["inferIn"], Primitive>, | ||
r = applyConstraint<t, Default<value>> | ||
>( | ||
value: value | ||
): instantiateType<r, $> | ||
): NoInfer<instantiateType<r, $>> | ||
default< | ||
const value extends this["inferIn"], | ||
r = applyConstraint<t, Default<value>> | ||
>( | ||
value: () => value | ||
): NoInfer<instantiateType<r, $>> |
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.
Seems good to use the same type util everywhere anyways
\o/ |
This did add 1 failing test and 1 not passing test
(because Dates are not immutable)