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

baseSchema.concat(otherSchema) overwrites baseSchema properties when otherSchema properties are undefined #1160

Closed
cemalettin-work opened this issue Dec 9, 2020 · 4 comments · Fixed by #1177
Labels

Comments

@cemalettin-work
Copy link
Contributor

cemalettin-work commented Dec 9, 2020

Describe the bug
Last_Edit: It is because of this code https://github.com/jquense/yup/blob/master/src/schema.ts#L231-L233 , combined.spec has every property undefined, it would work if combined.spec properties were non-existent instead of undefined:

const mergedSpec = { ...base.spec, ...combined.spec };

First_Edit: Looking a bit deeper, i am now not sure if it is caused by mixed.label(), but since the only thing we change is the yup version and this unexpected behavior occurs, i will keep this issue as is, hopefully someone experienced in yup can investigate or realize the reason why this happens.

Calling mixed.label("Custom label") does work on version 0.31.0 but not on 0.32.5+.
Example screenshots from v.0.31.0 and v.0.32.5+
v.0.31.0:

before_update

v.0.32.5+:

after_update

To Reproduce
Quite complex form logic react and formik, will try to extract the logic and edit here.

Expected behavior
I would expect the previous behavior of setting a label to work on error messages.

Platform (please complete the following information):

  • Browser [chrome]
  • Version [86]

Additional context
Using with react + formik

@cemalettin-work
Copy link
Contributor Author

cemalettin-work commented Dec 11, 2020

After spending some time on the issue i came to this conclusion ( codesandox had problems when testing so will leave it here):

const baseSchema = yup.mixed().label("Custom Label")...
const numberSchema = yup.number()... // some other methods which don't include .label()

const concattedSchema = baseSchema.concat(numberSchema)
// now the concattedSchema should have custom label in 0.31.0 but not 0.32.5+. Is this intended ?

@jquense
Copy link
Owner

jquense commented Dec 11, 2020

hmm no it's not intended. it should work like it used to.

@jquense jquense added the bug label Dec 11, 2020
@cemalettin-work
Copy link
Contributor Author

@cemalettin-work
Copy link
Contributor Author

cemalettin-work commented Dec 12, 2020

I coppied .label() call as a workaround, every field we had has a label but some of them were assigned type definitions if some other conditions were true, so that is why we were doing mixed.label() and concatting if condition :

const baseSchema = yup.mixed().label("Custom Label")...
const numberSchema = yup.number().label("Custom Label")... 

const concattedSchema = baseSchema.concat(numberSchema)
// now the concattedSchema has custom label label in all versions

@cemalettin-work cemalettin-work changed the title mixed.label() have different behavior after version 0.32 ( maybe only in nested schemas / form fields ? ) baseSchema.concat(otherSchema) overwrites baseSchema properties when otherSchema properties are undefined Dec 12, 2020
cemalettin-work added a commit to cemalettin-work/yup that referenced this issue Dec 13, 2020
Concatting schemas can overwrite previous non-undefined values with undefined
jquense pushed a commit that referenced this issue Dec 13, 2020
* fix(schema): Don't initialize spec values with undefined (#1160)

Concatting schemas can overwrite previous non-undefined values with undefined

* fix(schema): Add test cases for concatting and not overwriting (#1160)
cemalettin-work added a commit to NCI-Agency/anet that referenced this issue Dec 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants