Skip to content

chore: override broken @types package to avoid TS bump #427

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

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

dkundel
Copy link
Contributor

@dkundel dkundel commented Oct 6, 2022

Overriding a package that's incompatible with TS 3.9 (what we are currently using) as it's breaking the CI system. We should upgrade to the latest TypeScript major and pin @types packages respectively but that's a bigger effort.

Contributing to Twilio

All third-party contributors acknowledge that any contributions they provide will be made under the same open-source license that the open-source project is provided under.

  • I acknowledge that all my contributions will be made under the project's license.

@@ -38,7 +38,8 @@
"typescript": "^3.9.7"
},
"overrides": {
"@types/prettier": "2.6.0"
"@types/prettier": "2.6.0",
"@types/express-serve-static-core": "ts3.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these types needed here? It does not seem like this specific package.json has a dependency on Express. Perhaps this is just a part of how Lerna works that I'm not familiar with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think it's strictly necessary here. Npm overrides gives me a headache at times and wanted to be extra sure that the wrong version can't be anywhere in the path especially since we run TypeScript typically from the root. There's definitely some overall dependency cleanup we can do.

Copy link
Contributor

@bnb bnb left a comment

Choose a reason for hiding this comment

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

Are you okay with the override being relative? You're using a tag rather than a version so the version could change out from under you the future.

If so, this LGTM. If not, might want to pin to a specific version (4.17.28 is what the tag currently points to).

@dkundel
Copy link
Contributor Author

dkundel commented Oct 10, 2022

I'm trying to see if tags actually work for this pretty reliably so that in the future we can tag more @types dependencies with the tag rather than version range. If it doesn't work I'll revert to the fixed version.

@dkundel dkundel merged commit 2cda5b0 into main Oct 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants