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

fix: validator type imports for latest @types/validator #2360

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

benasher44
Copy link
Contributor

@benasher44 benasher44 commented Jan 10, 2024

Description

In DefinitelyTyped/DefinitelyTyped#68121, we fixed validator's types to improve compatibility with node16/nodenext. This fixes missing information that was important to users under certain configurations, but it unfortunately means that type imports in the repo need to be updated to be namespace imports (i.e. can no longer rely on the global validator namespace). This workaround is the way forward.

Checklist

  • the pull request title describes what this PR does (not a vague title like Update index.md)
  • the pull request targets the default branch of the repository (develop)
  • the code follows the established code style of the repository
    • npm run prettier:check passes
    • npm run lint:check passes
  • tests are added for the changes I made (if any source code was modified)
  • documentation added or updated
  • I have run the project locally and verified that there are no errors

Fixes

Closes DefinitelyTyped/DefinitelyTyped#68152
fixes #1866

@benasher44
Copy link
Contributor Author

@braaar from looking at the GitHub issues, it looks like this style of namespace import issue comes up every few years. This should fix it permanently for this repo.

@braaar
Copy link
Member

braaar commented Jan 11, 2024

This is great! I've been meaning to take a look at what's going on with the types that cause issues like #1866.

@braaar
Copy link
Member

braaar commented Jan 11, 2024

I notice you are not bumping the validator package itself. Isn't there a correspondence between versions of @types/valitor and validator that we should respect?

Copy link
Member

@braaar braaar left a comment

Choose a reason for hiding this comment

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

Since the checks are green, I see no reason to hold back on this. Thanks!

@braaar braaar changed the title Fix validator type imports for latest @types/validator fix: validator type imports for latest @types/validator Jan 11, 2024
@braaar braaar merged commit 0042559 into typestack:develop Jan 11, 2024
5 checks passed
@benasher44
Copy link
Contributor Author

I notice you are not bumping the validator package itself. Isn't there a correspondence between versions of @types/valitor and validator that we should respect?

Thanks for merging! Generally, yes. As it stands, this library doesn't use any new functions introduced since the declared version of validator. If you decide to introduce code that uses newer validator functionality, you'll need to bump the validator version to say that "hey if you're not using at least this newer version, it might crash". Since no new runtime functionality was introduced, I left the validator version alone, but I'm happy to open a new PR to bump it if you'd like!

For types, technically this library only needs the types from before this PR, since it only uses APIs/types from at least that version. However, this PR introduces a new goal wherein I want to show that when you update to at least this types version, there is an error, so then this PR can prove that error existed and then is fixed. To showcase that, I bumped the types version.

The new types version is backward compatible for the existing APIs you're already using in validator package, but you'll need to update the validator package version should you decide to broadcast usage of any of the newer APIs allowed by the added types in the newer types package version (though again happy to open a PR to preemptively upgrade).

I hope this makes sense!

Otherwise, might you be able to publish a new release? I think the folks on the linked ticket would appreciate it!

@benasher44 benasher44 deleted the basher/import-fix branch January 11, 2024 17:26
@benasher44
Copy link
Contributor Author

Saw the release — thank you!

Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 13, 2024
This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants