-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Update TypeScript version to use ~ #14387
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
Conversation
|
Hello @deyaaeldeen! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
| "rollup-plugin-visualizer": "^4.0.4", | ||
| "typedoc": "0.15.2" | ||
| "typedoc": "0.15.2", | ||
| "typescript": "~4.1.2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@witemple-msft this is a duplication (similar to what we have in the eslint plugin) but it should be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't doubt there's no issue with it, but is there a reason to keep it in both places?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No technical reason. I just like to keep the distinction between dependencies and devDependencies, TypeScript is needed as a package and also is needed as a tool to build in this case. If in the future we no longer need it as an imported package, we can remove its entry from dependencies and still have the devDependencies one for building.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels odd to me too, but I don't think it'll hurt anything?
| "rhea": "^1.0.24", | ||
| "rimraf": "^3.0.0", | ||
| "tslib": "^2.0.0", | ||
| "typescript": "4.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch
xirzec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this! 👍
maorleger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
This PR adds back a missing condition during the refactoring in Azure#14387
I confirmed with the TypeScript team that patch releases should not introduce breaking changes. This PR uses tilde in TypeScript version we use so get the latest patch releases for v4.1.