Skip to content
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

Merged
merged 11 commits into from
Sep 19, 2024
Merged

Conversation

Dimava
Copy link
Contributor

@Dimava Dimava commented Sep 18, 2024

This did add 1 failing test and 1 not passing test
(because Dates are not immutable)

Copy link
Member

@ssalbdivad ssalbdivad left a 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

ark/type/methods/base.ts Outdated Show resolved Hide resolved
ark/schema/structure/prop.ts Outdated Show resolved Hide resolved
ark/schema/structure/prop.ts Outdated Show resolved Hide resolved
ark/schema/structure/prop.ts Outdated Show resolved Hide resolved
ark/schema/structure/optional.ts Outdated Show resolved Hide resolved
ark/schema/structure/optional.ts Outdated Show resolved Hide resolved
ark/schema/structure/optional.ts Outdated Show resolved Hide resolved
ark/schema/roots/root.ts Outdated Show resolved Hide resolved

describe("works with objects", () => {
// it("default array in string", () => {
// const t = type({ bar: type("number[] = []") })
Copy link
Contributor Author

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/__tests__/defaults.test.ts Outdated Show resolved Hide resolved
ark/type/__tests__/defaults.test.ts Outdated Show resolved Hide resolved
ark/type/__tests__/defaults.test.ts Outdated Show resolved Hide resolved
ark/type/__tests__/defaults.test.ts Outdated Show resolved Hide resolved
ark/type/keywords/inference.ts Outdated Show resolved Hide resolved
ark/type/parser/tuple.ts Outdated Show resolved Hide resolved
ark/type/type.ts Outdated Show resolved Hide resolved
ark/type/__tests__/defaults.test.ts Outdated Show resolved Hide resolved
@Dimava
Copy link
Contributor Author

Dimava commented Sep 18, 2024

Hmm
image

@@ -434,6 +434,9 @@ export type Default<v = any> = {
default?: { value: v }
}

export type DefaultFor<t> =
t extends Primitive ? t | (() => t) : (t & Primitive) | (() => t)
Copy link
Contributor Author

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?

Copy link
Contributor Author

@Dimava Dimava Sep 18, 2024

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 []

Copy link
Member

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)

Copy link
Contributor Author

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

Copy link
Member

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.

@Dimava
Copy link
Contributor Author

Dimava commented Sep 19, 2024

So much cases, so much cases
" = x", [, "=", x], .default(x), type(, "=", x), all for x, ()=>x, make sure {x} doesn't work, make sure it's input checked and morphed, oof

@Dimava
Copy link
Contributor Author

Dimava commented Sep 19, 2024

Hmm,

type({ a: 'number' }).default({   |   }) 

doesn't have autocomplete, I don't like that

Comment on lines 143 to 154
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, $>>
Copy link
Contributor Author

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

Copy link
Member

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

export const assertDefaultValueAssignability = (
node: BaseRoot,
value: unknown,
key = ""
): unknown => {
const out = node.in(value)
if (!isPrimitive(value) && typeof value !== "function")
Copy link
Member

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/schema/structure/optional.ts Show resolved Hide resolved
Comment on lines 143 to 154
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, $>>
Copy link
Member

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

@ssalbdivad ssalbdivad changed the title feat: default factory feat: default value functions Sep 19, 2024
@ssalbdivad ssalbdivad merged commit 5f860bb into arktypeio:main Sep 19, 2024
6 checks passed
@Dimava
Copy link
Contributor Author

Dimava commented Sep 19, 2024

\o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done (merged or closed)
Development

Successfully merging this pull request may close these issues.

2 participants