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

fix: concat of mixed and subtype #444

Merged
merged 2 commits into from
Feb 6, 2019
Merged

Conversation

vonagam
Copy link
Contributor

@vonagam vonagam commented Feb 3, 2019

Separate PR from #443.

mixed().concat(number()).validateSync([]) // should throw

@vonagam
Copy link
Contributor Author

vonagam commented Feb 3, 2019

By the way, fixes #399 (in issue's example a problem in concating mixed and date, not in when).

@vonagam
Copy link
Contributor Author

vonagam commented Feb 4, 2019

Additional commit (can be separated into separate PR, but then i will have to make rebase after this PR will be merged). Small one, but fixes #381.

Smallest repo of a problem:

yup.object({
  conditional: yup.object().when("$check", {
    is: true,
    then: yup.object({
      lazy: yup.lazy(val => yup.number()),
    })
  }),
}).validateSync({}, {context: {check: true}});

It fixes it, but underlying cause of the issue still there:

  • validate in non-strict mode calls cast.

  • In object._cast field get resolved then casted.

  • cast calls resolve inside itself. So you end up with schema.resolve().resolve().

  • after resolve with conditions is executed resulted schema still contains original schema conditions, they are not stripped. So when you call resolve on resulted schema, those conditions will be called again and their result will be concated again.

  • and if in a result of conditions was a lazy schema, it ended up concating with itself, but Lazy do not have concat method and error was thrown.

Current solution solves the last point: skip merging same schema's or values.

Breaking addition will be a removal of original _conditions from resolve result. It will also allow to easily implement nested whens.

@vonagam vonagam mentioned this pull request Feb 4, 2019
src/util/merge.js Outdated Show resolved Hide resolved
src/util/merge.js Outdated Show resolved Hide resolved
src/util/merge.js Outdated Show resolved Hide resolved
@vonagam vonagam force-pushed the fix-concat-subtype branch 2 times, most recently from e250193 to 2978df1 Compare February 6, 2019 00:53
src/mixed.js Outdated Show resolved Hide resolved
src/mixed.js Show resolved Hide resolved
@jquense jquense merged commit 7705972 into jquense:master Feb 6, 2019
@jquense
Copy link
Owner

jquense commented Feb 6, 2019

thanks!

@vonagam vonagam deleted the fix-concat-subtype branch February 6, 2019 14:21
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

Successfully merging this pull request may close these issues.

2 participants