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.

Sign up for GitHub

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

Jump to bottom

Remove Joi from types #4497

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove Joi from types #4497

wants to merge 1 commit into from

Conversation

kanongil
Copy link
Contributor

@kanongil kanongil commented Apr 9, 2024 •
edited

This PR removes the explicit import of Joi in the typings. As it is, any typescript users that use Hapi, will need to add a dependency of Joi, just to resolve the typings.

The explicit dependency is replaced by a generic validator that can be set on the Server object, or inferred from the config. This has the additional advantage, that other validators will be able to be typed instead of assuming that Joi is used.

The new types will require a bit of work to adapt to for projects that use server.validate().

This fixes #4491.

Definition

const MySimpleValidator = {
    compile(schema: MySchema) {

        return {
            validate(value: unknown, options: MyOptions & { context?: Validation.Context }) {

                return { value: 'everything is awesome' };
            }
        };
    }
};

The base Joi object already implement this interface.

Registration

Explicit:

const server = new Server<ServerApplicationState, typeof Joi>({});
server.validator(Joi);

Inferred from config:

const server = new Server({
    routes: { validate: { validator: Joi } }
});

Using the new validator() returned value:

const server = new Server({})
    .validator(Joi);

For plugins

Explicit:

register(server: Server<unknown, typeof Joi>, options: any) {

    server.validator(Joi);
    …
}

Using the new validator() returned value:

register(_server: Server, options: any) {

    const server = _server.validator(Joi);
    …
}

Usage

Once registered, the validator typings are used to help type the validation options:

server.route({
    …,
    options: {
        validate: {
            options: {
                // Validated against the options that can be passed to the registered `validate()` method
            },
            query: {
                // Validated against the type of the `schema´ from the registered `compile()` method
            }
        }
    }
});

It can be overriden at the route level:

server.route({
    …,
    options: {
        validate: {
            validator: Joi,   // Route-only validator
            options: {
                // Validated against the options that can be passed to the validator `validate()` method
            },
            query: {
                // Validated against the type of the `schema´ from the validator `compile()` method
            }
        }
    }
});

It also allows custom inline validators:

server.route({
    …,
    options: {
        validate: {
            query: (value, optionsAndContext) {

                // optionsAndContext is typed with the `context` object
            }
        }
    }
});

It is also used to validate rules:

server.rules(processor, {
    validate: {
        schema: validateSchema,    // Validated against the type of the `schema´ from the registered `compile()` method
        options: {
            // Validated against the options that can be passed to the registered `validate()` method
        }
    }
});

As it currently is, pre-compiled Joi schemas continue to work, even with no registered validator (though any options won't be validated):

server.route({
    …,
    options: {
        validate: {
            query: Joi.object({
                …
            })
        }
    }
});

FYI, it's possible that this could be revised to extract the allowed options from the passed Joi object.

@kanongil kanongil added the types TypeScript type definitions label Apr 9, 2024
@Marsup
Copy link
Contributor

Marsup commented Apr 10, 2024

The PR makes sense to me, but applying it on my project causes a whole bunch of errors along the lines of Object literal may only specify known properties, and 'q' does not exist in type 'Validator<never> | DirectValidator<"query">'.. It appears I need to wrap all my object schemas (eg. validate: { query: { q: Joi.whatever() }}) into joi objects, I can't have simple objects anymore. Am I missing something?

@kanongil
Copy link
Contributor Author

Thank for the feedback. Unfortunately, this will likely take a bit of work to adapt to.

Specifying a validator is required to type such simple validation objects, since it can no longer default to Joi. You would need to use one of the above methods, or explicit casting, to make your Server object accept the Joi validator. You can also override it when specifying the route:

server.route<ReqRefDefaults, typeof Joi>({
    …
})

Of course, you can still just wrap it as suggested, though that would still lose the typings on any validate.options.

@Marsup
Copy link
Contributor

Marsup commented Apr 11, 2024

Considering the number of times a user would call server.route, can't we find a better way to define that with some declaration merging?

@kanongil
Copy link
Contributor Author

But you shouldn't declare it at the route level, but at the server level as in the "registration" section above.

If you are passing the server object to other methods that needs to call server.route() with validations, you will need to explicitly type the expected validator. Eg. function registerRoutes(server: Server<unknown, typeof Joi>) { … }.

I'm not sure what you mean by declaration merging? Do you mean to expect people to merge a DefaultValidator?

declare module '@hapi/hapi' {
    interface DefaultValidator extends Joi.Root {}
}

This will cause problems for independent plugins, which will either pollute the main project validator, or have to do without.

@Marsup
Copy link
Contributor

Marsup commented Apr 11, 2024

I'm torn, you're certainly right, but declaring independent plugins as I do, all the plugins will have to be changed to something like:

const plugin: Plugin<{}> = {
  register(server: Server<{}, typeof Joi>) {
  }
};

It doesn't look like a good DX but I'm not sure there's a better sane choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
types TypeScript type definitions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Build issues
2 participants
@kanongil @Marsup