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

.min or .destruct on .constructed fall back to main type #79

Open
phryneas opened this issue Nov 25, 2020 · 7 comments
Open

.min or .destruct on .constructed fall back to main type #79

phryneas opened this issue Nov 25, 2020 · 7 comments

Comments

@phryneas
Copy link
Contributor

Hey there again, sorry that I keep digging up weird stuff :)
I have created a constructed type with

const stringOrObjectWithKey = string.construct(
  (arg: { key: string } | string) =>
    typeof arg === "string" ? [arg] : [arg.key]
);

and I can call that just fine like

stringOrObjectWithKey("asd");
stringOrObjectWithKey({ key: "asd" });

now, when I chain that like

const withMin = stringOrObjectWithKey.min(1);

I lose the ability to call it with the overload:

withMin("asd");
// Argument of type '{ key: string; }' is not assignable to parameter of type 'string'.ts(2345)
withMin({ key: "asd" });

Looking at the types, I don't really see what is wrong there - and to be honest, these types are quite complicated to get started with so I'm a bit hesitant to do a PR on this.

I have, though, created a type test for this that you can use to verify this behaviour:
CodeSandBox

I hope that helps to pinpoint it down.

Please don't hesitate to contact me if I can help any further 🙂

/*eslint-disable @typescript-eslint/no-unused-vars */

import Schema, { string } from "computed-types";
import { SchemaResolveType, SchemaInput } from "computed-types/lib/schema/io";

function expectType<T>(t: T) {
  return t;
}
function expectAssignable<Subtype, _Type extends Subtype>() {}

const stringOrObjectWithKey = string.construct(
  (arg: { key: string } | string) =>
    typeof arg === "string" ? [arg] : [arg.key]
);

stringOrObjectWithKey("asd");
stringOrObjectWithKey({ key: "asd" });
expectType<(v: string | { key: string }) => string>(stringOrObjectWithKey);
expectAssignable<
  string | { key: string },
  SchemaInput<typeof stringOrObjectWithKey>
>();
expectAssignable<
  SchemaInput<typeof stringOrObjectWithKey>,
  string | { key: string }
>();

const destructed = stringOrObjectWithKey.destruct();
destructed("asd");
destructed({ key: "asd" });
expectType<(v: string | { key: string }) => [any, string?]>(destructed);
// second signature missing here
expectType<(v: string) => [any, string?]>(destructed);
expectAssignable<string | { key: string }, SchemaInput<typeof destructed>>();
expectAssignable<SchemaInput<typeof destructed>, string | { key: string }>();
expectAssignable<[any, string?], SchemaResolveType<typeof destructed>>();
expectAssignable<SchemaResolveType<typeof destructed>, [any, string?]>();

const withMin = stringOrObjectWithKey.min(1);
withMin("asd");
withMin({ key: "asd" });
expectType<(v: string | { key: string }) => string>(withMin);
// second signature missing here
expectType<(v: string) => string>(withMin);
expectAssignable<string | { key: string }, SchemaInput<typeof withMin>>();
expectAssignable<SchemaInput<typeof withMin>, string | { key: string }>();
expectAssignable<string, SchemaResolveType<typeof withMin>>();
expectAssignable<SchemaResolveType<typeof withMin>, string>();

const inSchema = Schema({ test: stringOrObjectWithKey });
inSchema({ test: "asd" });
inSchema({ test: { key: "asd" } });
expectType<(v: { test: string | { key: string } }) => { test: string }>(
  inSchema
);
expectAssignable<
  { test: string | { key: string } },
  SchemaInput<typeof inSchema>
>();
expectAssignable<
  SchemaInput<typeof inSchema>,
  { test: string | { key: string } }
>();

const destructedSchema = inSchema.destruct();
destructedSchema({ test: "asd" });
destructedSchema({ test: { key: "asd" } });
expectType<(v: { test: string | { key: string } }) => [any, { test: string }?]>(
  destructedSchema
);

const mergedSchema = Schema.merge({ other: string.optional() }, inSchema);
mergedSchema({ test: "asd" });
mergedSchema({ test: { key: "asd" } });
expectType<(v: { test: string | { key: string } }) => { test: string }>(
  mergedSchema
);
expectAssignable<
  { test: string | { key: string } },
  SchemaInput<typeof mergedSchema>
>();
expectAssignable<
  SchemaInput<typeof mergedSchema>,
  { test: string | { key: string } }
>();

const mergedDestructedSchema = mergedSchema.destruct();
mergedDestructedSchema({ test: "asd" });
mergedDestructedSchema({ test: { key: "asd" } });
expectType<(v: { test: string | { key: string } }) => [any, { test: string }?]>(
  mergedDestructedSchema
);
@phryneas
Copy link
Contributor Author

phryneas commented Nov 27, 2020

For anyone with a similar problem:

For now, I'm falling back to

export const stringOrObjectWithKey = unknown.transform((input: any | string | { key: string }) => {
    if (typeof input === 'string') return input;
    if (input && typeof input.key === 'string') return input.key;
    throw new TypeError(`Expected string or { key: string }: "${input}"`);
}, StringValidator) as ValidatorProxy<StringValidator<[string | { key: string }]>>;

which works as expected.

But in the long run, it would be nice if .construct were correctly chainable :)

PS:

export const stringOrObjectWithKey = Schema.either(string, Schema({ key: string })).transform(
    (input) => (typeof input === 'string' ? input : input.key),
    StringValidator
) as ValidatorProxy<StringValidator<[string | { key: string }]>>;

also seems to do the trick and leaves a bit more work to computed-types

@moshest
Copy link
Member

moshest commented Nov 27, 2020

It's impossible to detect on runtime what's the right chain methods from the object types. This is why when it's not known (like on destruct) it will use the main type by design.

It will be much easier to try help you with this issue if you give me some real world use-case. Currently it's very hard to understand what are you trying to achieve here.

@moshest moshest added the awaiting feedback More data is needed before taking any more actions label Nov 27, 2020
@phryneas
Copy link
Contributor Author

phryneas commented Nov 27, 2020

Sure 👍
I have a type of input that can either be of the form { key: "something" } from a dropdown, or just a string from an input field.

In some cases, I want to validate that that input/dropdown value has a minimum length of 1 (so it isn't the "non-selected" case) and in other cases, I want it to be optional and also transform the empty string to undefined for further processing.

So, one use would be stringOrObjectWithKey.min(1) and another would be stringOrObjectWithKey.optional().transform(emptyToUndefined), with

export function emptyToUndefined(s: string | undefined | null) {
    return !s ? undefined : s;
}

I had interpreted string.construct(coercion) as "add additional input types that are coerced to string before further validation/transformation is done", but I might also just have misinterpreted the documentation there, because right now every validation/transformation I try to chain on resets the input type to plain string.

PS: I also want this Schema to accept both string and { key: string } because this way I can validate & transform on the client and also do a second validation on the server with the transformation output. I'm using a graphql server, so validate(client) -> validate(server) -> transform(server) is not possible, because the server already accepts a very specific input format - transformation has to happen on the client.

@moshest
Copy link
Member

moshest commented Nov 28, 2020

Looks like you need to use Schema.either:

const Validator = Schema.either(
  string.min(1),
  Schema({ key: string }),
);

You can add more logic and constraints for each type individually.

@moshest moshest added question Further information is requested and removed awaiting feedback More data is needed before taking any more actions labels Nov 28, 2020
@phryneas
Copy link
Contributor Author

My idea was to convert both parts of that either to a string (because I definitely want the output to be transformed to a string in the end, I just have multiple possible input shapes) so I don't have to do

const Validator = Schema.either(
  string.min(1).all().the().other().validations(),
  Schema({ key: string.min(1).all().the().other().validations() }),
);

but instead can do

const Validator = stringOrObjectWithKey.min(1).all().the().other().validations()

But starting with an either is a good idea and I'm starting to get a handle of the second argument to transform (and how to type it) now, so I'm ending up with

export const stringOrObjectWithKey = Schema.either(string, Schema({ key: string })).transform(
    (input) => (typeof input === 'string' ? input : input.key),
    StringValidator as ValidatorConstructor<StringValidator<[string | { key: string }]>>
);

It would be cool if you could add some documentation to the second argument of transform to the docs, as right now it's quite a journey to learn how to transform from one type to another 😅

My first instinct of "construct a string from these two types and continue working on that as if it were a string" seems to have been very much off the mark ^^

@moshest
Copy link
Member

moshest commented Nov 30, 2020

If on the end you want a string then it make sense to use construct, but it's true that the types there may have some bugs getting that.

TypeScript making it very hard to mange that. This library always feels as we pushing TypeScript to it limits. Maybe on the new 4.1 version it will be much easier to address that, but it will also involve breaking change for older versions.

I will take a deeper look on this when I have some time.

@moshest moshest removed the question Further information is requested label Nov 30, 2020
@phryneas
Copy link
Contributor Author

TypeScript making it very hard to mange that. This library always feels as we pushing TypeScript to it limits. Maybe on the new 4.1 version it will be much easier to address that, but it will also involve breaking change for older versions.

Yeah, I know the feeling - I'm a co-maintainer of redux toolkit, and boy, do we have some types.(Including stuff like feature-detecting the TypeScript version and acting different on that 😅)
That's why I provided the type test in the first post - so you can fiddle around with the types and see if stuff continues working or breaks while you make changes. Writing these has been of great value to me, both for identifying the real culprit as well as for fixing it - maybe they help you, too 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants