deps(types-node): avoid formal dependency on types-node22 #5801
Conversation
Hweinstock
changed the title
Hweinstock
changed the title
Co-authored-by: Justin M. Keyes <[email protected]>
| "webpack-merge": "^5.10.0" | ||
| }, | ||
| "dependencies": { | ||
| "@types/node": "^22.7.5", |
There was a problem hiding this comment.
Does it work if this is 18 instead of 22?
I'm confused about the relationship of the copyFiles.ts scripts vs the packages/x/package.json declarations.
As an alternative to fs as any, maybe we could use the techdebt.test.ts reminder to remind us to remove this top-level dependency when the getMinNodejsVersion result is new enough?
There was a problem hiding this comment.
Looks like that works as well, I can update the techdebt test to match. I thought avoiding the dependency was more important than the any workaround. But I guess the version bump isn't something to worry about? Are there any other tradeoffs to consider?
There was a problem hiding this comment.
I thought avoiding the dependency was more important than the
anyworkaround.
Since this PR makes the total cost clearer (i.e.: small :), I'm having doubts. And also, since the risk is covered by techdebt.test.ts in either case, perhaps it's better to have type checking.
Either way is fine IMO.
There was a problem hiding this comment.
I think I'll keep the dependency then. Since its just a version bump (to an old stable version) and not a whole new thing, the cost seems very low. The any code doesn't read well, and could cause confusion to someone without the necessary context.
…scode into removeNodeTypes
## Problem Follow up to: aws#5761 (comment) ## Solution - target a version of `@types/node` that is closer to the actual version running in CI. - add a techdebt.test.ts reminder.
2 participants
Problem
Follow up to: #5761 (comment)
Solution
@types/node@22usinganyworkaround.anyworkaround once we bump to node 18 in CI.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.