Skip to content

Conversation

@deadlycoconuts
Copy link
Contributor

@deadlycoconuts deadlycoconuts commented Aug 2, 2024

Context

It seems like after upgrading yup from 0.29.1 to 1.4.0, we have broken the its compatibility with the @overgear/yup-ast package. This package is ancient; it's last published 6 years ago and no longer receives any updates because the original contributors have left the company and do not have any permissions/licenses to continue contributing to them. Their replacement is the @demvsystems/yup-ast package, which works almost exactly the same as before.

This package is used when parsing the yup-like schema that's served over an API server endpoint (see https://github.com/caraml-dev/xp/blob/cb06edaaf409077b5abf7fed0b803984b6decf7e/plugins/turing/manager/experiment_manager.go#L38C1-L38C36). We only use this package in one place, when parsing the yup-like schema off the experiment engines endpoint on the Turing API server (this in turn gets its value from the XP experiment manager plugin, which explains why the file quoted above is found in the XP repository and not in Turing). This parsed schema is then used to validate the XP experiment engine config together with the other yup schemas of the Turing router config.

More concretely, this broken package is currently causing this problem whereby the yup validation that gets triggered when a user clicks on the 'Next' page of an accordion form, to end up permanently stuck:
image-20240801-142115

A separate PR in XP has also been created to fix a related bug in the yup-like schema quoted above: caraml-dev/xp#81

Copy link
Collaborator

@tiopramayudi tiopramayudi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! Thanks @deadlycoconuts

@deadlycoconuts
Copy link
Contributor Author

Thanks a lot for the quick review @tiopramayudi ! Merging this now!

@deadlycoconuts deadlycoconuts merged commit c1924b4 into caraml-dev:main Aug 5, 2024
@deadlycoconuts deadlycoconuts deleted the replace_yup_ast branch August 8, 2024 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants