Skip to content

Remove support for pre v16 joi#4474

Open
kanongil wants to merge 1 commit into
hapijs:nextfrom
kanongil:less-joi
Open

Remove support for pre v16 joi#4474
kanongil wants to merge 1 commit into
hapijs:nextfrom
kanongil:less-joi

Conversation

@kanongil

@kanongil kanongil commented Nov 17, 2023

Copy link
Copy Markdown
Contributor

The last joi@15.1.1 was released more than 4 years ago. It should be safe to drop (in a breaking release).

This also removes several npm deprecation notices when installing dev dependencies.

@kanongil kanongil added the breaking changes Change that can breaking existing code label Nov 17, 2023
@damusix

damusix commented Nov 28, 2024

Copy link
Copy Markdown
Contributor

@kanongil Is this still a valid PR? Any chance you can update it if need be? We're converging for a major release and may include this if it's ready.

@kanongil

Copy link
Copy Markdown
Contributor Author

There might be an issue with this PR, let me check…

@kanongil kanongil left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We need to keep support for non-async validators, since an external validator might only use this.

Ie. I have removed the logic changes, and this patch just replaces the legacy joi test, with a custom non-async validator test.

The test failures seem unrelated to this PR. Note that node@14 also a fails while reporting a false positive.

@kanongil kanongil added test Test or coverage and removed breaking changes Change that can breaking existing code labels Nov 29, 2024
@kanongil

Copy link
Copy Markdown
Contributor Author

Note that since this no longer changes the logic, this patch could also be applied against the main branch.

@damusix

damusix commented Jan 19, 2025

Copy link
Copy Markdown
Contributor

@kanongil Can you rebase and fix the lint errors on this?

Screenshot 2025-01-19 at 1 53 29 AM

@kanongil

Copy link
Copy Markdown
Contributor Author

No – It is already based of the tip of next. I have made a new PR against master, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test Test or coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants