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

Feature: Marking options as deprecated #1957

Closed
shellscape opened this issue Jun 30, 2019 · 6 comments
Closed

Feature: Marking options as deprecated #1957

shellscape opened this issue Jun 30, 2019 · 6 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@shellscape
Copy link

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

I'm working on a fork(ish) of a module that has been put into "stasis," meaning that it is stable but will not receive any updates beyond security fixes. Part of my fork is an effort to update the codebase and redefine options. However, I'd like to maintain a compatibility layer for the upstream options set. The kick is that I'd also like to mark those upstream options as deprecated so users of the fork move to the redefined options for a smooth transition.

It occurred to me that joi might be a good place for consolidating that kind of check. So I bring this idea to you all for feedback.

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

Perhaps a deprecated() method could be introduced. That could produce messaging, or return a particular error+code for handling deprecated options.

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

Absolutely. But I'd like to get some feedback before I dive in.

@hueniverse
Copy link
Contributor

We don't have a concept of warnings. Validation is either successful or not. I am not even sure how such warnings should be reported back. Are you thinking of using this in an API or as part of HTTP-based services response?

@hueniverse hueniverse self-assigned this Jun 30, 2019
@shellscape
Copy link
Author

An API (npm modules that I published which have user options). The pattern of success/failure can still be adhered to. My idea was to throw a particular class of Error and set the code (which is what Node recommends these days) to indicate that the error is a deprecation error. Consumers of joi could then decide to trap that particular error and report to the user as desired.

@hueniverse
Copy link
Contributor

So this is not a warning but a specific kind of error? If that's the case, you can just use .error() and pass the deprecation error you want.

@shellscape
Copy link
Author

shellscape commented Jun 30, 2019

@hueniverse I'm pretty disappointed this was closed without further discussion. I don't believe you've fully understood the distinction I'm speaking about. I'd like to contribute but the closure was awfully abrupt. (Or perhaps I haven't understood what you're suggesting)

@hueniverse
Copy link
Contributor

First, I close issues pretty fast. It doesn't mean you can't continue the conversation...

There are two ways to look at your request.

One is that you want some kind of warning when a deprecated input is passed. You still want validation to be successful, and maybe even use rename() to fix the old option to the new one. This means you want to generate certain rules that instead of creating an error, they create a warning. Then all the warnings are collected and reported back alongside the value and error properties. I don't know where to put these when you use the promise API since we reject the error and resovle the value. Only option there is to add a new config option that replaces value with { value, warnings }.

The second is that you do want validation to fail, but you want to identify the reason as deprecation related. In that case, you do not need anything new. You can simply specify the key as forbidden().error(err) and set err to the "This option has been deprecated" error you want to generate.

I first thought you were asking for the first, but they your response made me believe you are asking for the second, in which case, you don't need anything new.

@hueniverse hueniverse reopened this Jul 1, 2019
@shellscape
Copy link
Author

Thank you very much for being so verbose and for reopening. I really appreciate the longer explanation of the second option - the shortcoming was mine as I hadn't comprehended what you were suggesting. I believe that the forbidden().error(err) route will suit me fine and will roll with that for now.

@hueniverse hueniverse mentioned this issue Jul 1, 2019
@hueniverse hueniverse added this to the 16.0.0 milestone Aug 11, 2019
@hueniverse hueniverse added the v16 label Aug 11, 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

2 participants