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
feat(types): drop TypeScript < 4.1 #13954
Conversation
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.
Small suggestion for additional clarity
Also, do we really want this in v6? |
This in v6 would allow us to merge #13909 in v6 which would provide an easier migration path to the model changes I'd like to make in v7 (simplying model definition to But I am also of two minds about it |
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
I think it will be backward incompatible for some users right? how about adding it to v7 instead? |
It will be incompatible for users that haven't updated TypeScript since Nov. 19 2020 yes. Which is why I'm of two minds. On one hand, adding this to v6 would allow us to land #13909 in v6 which would be a great improvement for TS users & make migration to v7 easier. And 4.1 is more than one year old. On the other hand, But honestly, I have typings errors all the time after upgrades. Even Sequelize adding proper types for Honestly my only problem with this change is that we didn't document it earlier. But I think given it's been more than a year since 4.1, it's fine. Users that haven't updated TS since then are not going to update Sequelize either. |
Yeah, it's more about that we have a policy now relatively close to a new major release. But I agree that with the changes we made to the typings many people that have updated to the latest version of sequelize also have a relatively new version of TS as well. If migrating to v7 is made easier with this change I think it's fine if we do this in v6 |
If this makes our work easier for migrate typings, approve! |
It won't make our job easier but it will allow users to massively reduce the boilerplate they need to write for TS support in v6 Current way: https://github.com/sequelize/sequelize/blob/main/docs/manual/other-topics/typescript.md#usage Which would also make migration easier as that second way is what I'd like to make the default in v7 |
I'll merge this because there is consensus that we should support >= 4.1 in v7 ; but we can still debate whether we land it in v6 or not :) Edit: If I understand correctly we first need to mark tests |
Thanks @sdepold :') |
Seeing the difference in usage I think it's good to include this in v6 as well. Seeing that we have updated various typings in the recent releases of v6 already it should be fine.
I updated the required checks so it requires 4.1 - 4.5 already for the main branch |
What's the benefit of having this in v6? |
@sdepold this is the benefit of having it in v6 EDIT: so not this PR specifically, but it allows us to merge other PRs to v6 as well which will accomplish at least the example quoted |
Cool! |
Ah sorry. My Mumbo jumbo brain somehow thought that this thread is about the utils conversion. I think we should change the docs part in the commit message to something else. Maybe Feature for the lack of a better idea since docs is likely to result in a patch version |
|
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
🎉 This PR is included in version 7.0.0-alpha.6 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>
Pull Request Checklist
Please make sure to review and check all of these items:
npm run test
ornpm run test-DIALECT
pass with this change (including linting)?Follow-up to sequelize/meetings#6 (comment)
This PR drops support for TypeScript < 4.1 and documents our TS support policy.
This release should cause a SemVer MINOR bump.