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

Add peerDependency warning if using < typescript 5 #359

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

EliBates
Copy link

@EliBates EliBates commented Jan 8, 2024

I discovered this issue using nextjs when when using typescript < 5.
#352

This adds a peerDependency of typescript 5 so that anyone installing valibot will be warned if they don't have the appropriate version of TypeScript installed in their project.

@fabian-hiller
Copy link
Owner

fabian-hiller commented Jan 8, 2024

The idea is great, but there is one problem. If someone is not using TypeScript, this change will erroneously display a warning when installing Valibot.

@fabian-hiller fabian-hiller self-assigned this Jan 8, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label Jan 8, 2024
@EliBates
Copy link
Author

EliBates commented Jan 9, 2024

@fabian-hiller Good point. I think we can add:

  "peerDependenciesMeta": {
    "typescript": {
      "optional": true
    }
  }

This should make it warn only users who are using typescript. I think this will be helpful because I still see a lot of projects on <5 typescript.

@fabian-hiller fabian-hiller added enhancement New feature or request priority This has priority and removed question Further information is requested labels Jan 9, 2024
@fabian-hiller
Copy link
Owner

Thank you! I will review this PR in the next few days.

Comment on lines +13 to +15
"peerDependencies": {
"typescript": ">=5"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

One thing to note about the TypeScript project is that it does not follow SemVer, and breaking changes can be introduced in minor versions.

You might want to be more explicit about versions that are compatible with the tilde syntax:

Suggested change
"peerDependencies": {
"typescript": ">=5"
},
"peerDependencies": {
"typescript": "~5.0||~5.1||~5.2||~5.3"
},

Or even just a more specific range:

Suggested change
"peerDependencies": {
"typescript": ">=5"
},
"peerDependencies": {
"typescript": "=5"
},

@fabian-hiller
Copy link
Owner

Sorry for not picking up this PR yet. Other things had a higher priority. I expect to review it this month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority This has priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants