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

Date ref not does not appear to be getting cast #553

Closed
akmjenkins opened this issue Jun 17, 2019 · 6 comments
Closed

Date ref not does not appear to be getting cast #553

akmjenkins opened this issue Jun 17, 2019 · 6 comments

Comments

@akmjenkins
Copy link
Contributor

akmjenkins commented Jun 17, 2019

I've got some utility functions for yup that I use like so:

export const after = (value, message, schema = yup.date()) => schema.clone().min(value, message);

This works:

after('2010-01-01', message).isValidSync('2012-01-01') // true

This does not:

yup.object({
    myField: afterTwo(yup.ref('field2'), message),
}).isValidSync({
    myField:'2012-01-01',
    field2:'2010-01-01'
})

From what I can tell in the test, it looks like it resolves the ref correctly, but it doesn't cast it (at least not in the way I think it should cast it, but I could be misunderstanding something).

I can fix it by doing this, (but it's much less elegant :( ):

export const after = (value, message, schema = yup.date()) => {
  return schema.clone().test({
    name: 'after',
    message,
    test: function(against) {
      return this.resolve(against) > schema.cast(this.resolve(value));
    },
  });
};

I'm just wondering if this is a bug that the resolved ref isn't getting cast in the first example I posted? Am I just misunderstanding how casting is supposed to work (completely possible)? Is it at all related to this old issue: #40?

@jquense
Copy link
Owner

jquense commented Jun 18, 2019

On the face of it, it would make sense it doesn't cast, b/c there is no schema definition for field2 to use to cast it. Agree the current behavior isn't all that helpful tho.

also

schema.clone().min(value, message);

you don't need the clone() in your helper, any action of a schema will clone automatically b/c they are immutable

@akmjenkins
Copy link
Contributor Author

@jquense Thanks for the tip! Would it make sense to pass a schema or cast function as an option to the ref? Right now, ref's only take contextPrefix as an option, I could envision an api like this:

yup.object({
    myField: afterTwo(yup.ref('field2',{cast:yup.date().cast}), message),
}).isValidSync({
    myField:'2012-01-01',
    field2:'2010-01-01'
})

Not sure if it would help to define the whole schema for it like:

yup.object({
    myField: afterTwo(yup.ref('field2',{schema:yup.date()}), message),
}).isValidSync({
    myField:'2012-01-01',
    field2:'2010-01-01'
})

I'd love to be a contributor to such an awesome library. If you like the concept I could submit a PR?

@akmjenkins
Copy link
Contributor Author

akmjenkins commented Jun 19, 2019

Just wanted to add one further thing. If we could access the parent schema - see #551 - not just the parent object, then, at least for sibling ref's, we wouldn't even have to pass in a cast/schema option, to the ref, we could just infer it from the schema type of the sibling (invaluable feature, I think!). We would still have to supply a schema/cast option if the ref were context, though.

@jquense
Copy link
Owner

jquense commented Jun 20, 2019

we could just infer it from the schema type of the sibling

how would that work there since isn't a sibling schema, if there was it'd be used already. This example is a bit weird b/c you are using throw-away data in the value input as a dependency for a known field

@akmjenkins
Copy link
Contributor Author

we could just infer it from the schema type of the sibling

how would that work there since isn't a sibling schema, if there was it'd be used already. This example is a bit weird b/c you are using throw-away data in the value input as a dependency for a known field

Sorry! Still learning the library!

@tdozi
Copy link

tdozi commented Feb 24, 2020

My issue with this is Date.min() docs suggest that the limit(string) gets cast. Why wouldn't Ref that's a string in context also get cast to a date? Otherwise, what's the suggested way to cast contextual References.

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

3 participants