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

Allow self references #1569

Closed
james-rabbit opened this issue Aug 17, 2018 · 4 comments
Closed

Allow self references #1569

james-rabbit opened this issue Aug 17, 2018 · 4 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@james-rabbit
Copy link

james-rabbit commented Aug 17, 2018

Describe the problem you are trying to fix (provide as much context as possible)

I use a success boolean key in message responses, and the structure of the message often depends on if it is true or false. I use Joi.alternatives to match on true or false but the syntax is pretty awkward and confusing for new readers (especially unknown()):

responseSchema: Joi.object({
	messageId: Joi.number(),
    success: Joi.boolean(),
).when(Joi.object({ success: true }).unknown(), {
	then: Joi.object({
		responseObject: Joi.object(),
		otherData: Joi.string(),
		isDataNew: Joi.boolean(),
	}),
	otherwise: Joi.object({
		reason: Joi.string().valid(Object.values(Constants.FailureReasons)),
		description: Joi.string().optional(),
	}),
}),

The line I think is strange is this one:

).when(Joi.object({ success: true }).unknown(), {

Which API (or modification of the current API) do you suggest to solve that problem ?

I would prefer a syntax that supported this case more easily and clearly.
I am not sure what the ideal solution would be, but maybe something like this would work:

responseSchema: Joi.object({
	messageId: Joi.number(),
    success: Joi.boolean().case({
        true: Joi.object({
		    responseObject: Joi.object(),
		    otherData: Joi.string(),
		    isDataNew: Joi.boolean(),
	    }),
        false: Joi.object({
		    reason: Joi.string().valid(Object.values(Constants.FailureReasons)),
		    description: Joi.string().optional(),
	    }),
    }),
),

Are you ready to work on a pull request if your suggestion is accepted ?

Honestly I am not sure I have the depth of understanding required to contribute to this project yet, I am just getting into it.

@Marsup Marsup added the request label Aug 17, 2018
@Marsup
Copy link
Collaborator

Marsup commented Aug 17, 2018

This syntax would only work for strings though, and how do you define the default case ?

@james-rabbit
Copy link
Author

james-rabbit commented Aug 17, 2018

I was only thinking of defining it for booleans really. The schema definition is powerful and I wouldn't want to define a whole parallel syntax to it.

I think the main thing that makes me feel like it's not already simple is the unknown(). If it was allowing unknown values for the when match by default then it could just be
).when({ success: true }, {
which is clear and has all the power of the schema.

Could this be solved by a validate-level option, something like allowUnknownInAlternatives?

@hueniverse hueniverse changed the title Request: Simpler alternatives syntax Allow self references Nov 11, 2018
@hueniverse hueniverse self-assigned this Nov 11, 2018
@hueniverse
Copy link
Contributor

responseSchema: Joi.object({
    messageId: Joi.number(),
    success: Joi.boolean(),
)
    .when('.success', {
        is: true,
        then: {
            responseObject: Joi.object(),
            otherData: Joi.string(),
            isDataNew: Joi.boolean()
        },
        otherwise: {
            reason: Joi.string().valid(Object.values(Constants.FailureReasons)),
            description: Joi.string().optional()
        }
    });

@Marsup Marsup closed this as completed in 32a649f Nov 12, 2018
Marsup added a commit that referenced this issue Nov 12, 2018
Support self references. Closes #1569
@james-rabbit
Copy link
Author

I like the new self reference syntax, thank you @hueniverse!

@hueniverse hueniverse added this to the 16.0.0 milestone Aug 10, 2019
@hueniverse hueniverse added the v16 label Aug 10, 2019
@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

3 participants