workspace: missing dependencies
Expected Behavior
All dependencies used by a package should be declared in that packages package.json file.
This ensures a safe package resolution & install process independent from the used package manager.
Actual Behavior
Certain dependencies in different packages are missing an explicit declaration in their respective package.json.
Complete List of missing dependencies by package:
packages/@cdktf/cli-core
ci-infois missing (usage is in./src/lib/error-reporting.ts)
packages/@cdktf/commons
strip-ansiis missing (usage is in./src/util.ts)
packages/@cdktf/hcl2cdk
fs-extrais missing (usage is in./test/helpers/convert.ts--> so should be adevDependencyprobably?)execais missing (usage is in./test/helpers/convert.ts--> so should be adevDependencyprobably?)codemakeris missing (usage is in./lib/coerceType.ts)
packages/@cdktf/provider-generator
globis missing (usage is in./lib/get/constructs-maker.ts)
packages/@cdktf/provider-schema
deepmergeis missing (usage is in./src/read.ts)
I stumbled across this issue due to our pipeline sometimes failing during a cdktf deploy with error messages like:
Cannot find module 'strip-ansi'
...
Cannot find module 'glob'
...
etc.
As we use turborepo + pnpm (which I know is not officially supported by cdktf) and you have not stumbled over those missing dependency declarations so far my guess is that yarn / lerna workspaces are in some way more forgiving wrt. these declarations (compared to pnpm).
Besides fixing the issue for our (to be fair unsupported pnpm) usecase I nevertheless think it would be sensible to declare the dependencies explicitly as mentioned above.
Steps to Reproduce
- Clone the repository.
- Go into any of the mentioned packages:
packages/@cdktf/cli-corepackages/@cdktf/commonspackages/@cdktf/hcl2cdkpackages/@cdktf/provider-generatorpackages/@cdktf/provider-schema
- Run
npx depcheck(See depcheck) and look at the "Missing dependencies" section
Versions
I cloned the repository on commit: e207a50aae198ac7cc78dc40356b35946c9d1ad8 (origin/main at that time)
Tags: v0.19.0-pre.34, v0.19.0
Providers
No response
Gist
No response
Possible Solutions
Add the missing dependencies as explicit declarations in the respective package.json files.
Workarounds
Anything Else?
Please let me know if you need anything else from me, I'll be glad to support further to get the issue resolved.
References
No response
Help Wanted
- [X] I'm interested in contributing a fix myself
Community Note
- Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
- Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
- If you are interested in working on this issue or have submitted a pull request, please leave a comment
Thank you for pointing this out, I opened a PR to fix this and added one that runs depcheck on PRs so we don't run into this issue again :) yarn is very convenient, but it makes it easy to forget adding the dependencies when everything runs as expected
No worries! Great to hear, thanks for reacting so quickly 🙂
yarn is very convenient, but it makes it easy to forget adding the dependencies when everything runs as expected
Just enable Yarn PnP and you will see all possible dependency issues, including require / dynamic imports )
BTW, there is additional missed dependency @cdktf/node-pty-prebuilt-multiarch in cdktf-cli. It's not a direct dependency, so you can't find any import / require in code. But, it present in the compiled bundle/bin/cmds/handlers.js:
The root cause is: https://github.com/hashicorp/terraform-cdk/blob/e207a50aae198ac7cc78dc40356b35946c9d1ad8/packages/cdktf-cli/build.ts#L72-L82 package.json doesn't contains that dependency.