Add @types/node as a dev dependency for TypeScript projects

[ghost]

Add @types/node as a dev dependency for TypeScript projects as this is needed to access the process object used for getting environment vars.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[mergify]

Thanks so much for taking the time to contribute to the AWS CDK ❤️

We will shortly assign someone to review this pull request and help get it
merged. In the meantime, please take a minute to make sure you follow this
checklist:

• PR title type(scope): text
  • type: fix, feat, refactor go into CHANGELOG, chore is hidden
  • scope: name of module without aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
  • text: use all lower-case, do not end with a period, do not include issue refs
• PR Description
  • Rationale: describe rationale of change and approach taken
  • Issues: indicate issues fixed via: fixes #xxx or closes #xxx
  • Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>
• Testing
  • Unit test added. Prefer to add a new test rather than modify existing tests
  • CLI or init templates change? Re-run/add CLI integration tests
• Documentation
  • README: update module README to describe new features
  • API docs: public APIs must be documented. Copy from official AWS docs when possible
  • Design: for significant features, follow design process

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[eladb]

I am okay with adding this despite the fact that it is not normally needed. @RomainMuller what do you think? You were the one that removed them earlier.

[jogold]

If I might... I would say that as soon as you start working with assets you usually use path.join() (importing path) and then @types/node is required...

[RomainMuller]

I wouldn't do this at all, as this has been a source of issues in the past, and to a certain extent it takes control away from the use:

• If we proactively configure this, then we're also possibly restricting the set of node standard library features that are available (only those up to the version of @types/node we choose will be available, but the user may elect to run with a much more recent node engine).
• Also, this has caused issues in the past in two ways (resulting in tsc errors that are hard to understand, but are easier to debug when you know it broke right after you added a new dependency...):
  • When we used ^8.x.y, we occasionally got broken by new & broken versions of this package
  • When we used 8.x.y (no caret), we hit issues when some type definitions were moved over to TypeScript's ES#### libraries (because of duplicate declarations of the same type name)

I we however feel that using process.env is common enough to warrant being proactive about it, we must use the version of @types/node that matches our minimum node engine requirement (currently, this would be ^8.10.53), otherwise you're advertising features of the node standard library that may, in fact, not be available.

[ghost]

Would that it were so simple, I guess. I didn't run into versioning issues myself because I have a near-current Node.js installed, so of course the latest @types/node module works perfectly with it. The note in the Dev Guide will have to do for now.

Maybe there's a way we could provide the necessary type info for just process in one of the CDK packages, assuming that hasn't changed a lot since our minimum version of Node?

Anyway, this isn't the PR we're looking for, so I'll close it.

[eladb]

@RomainMuller and I discussed this. @RomainMuller can you please follow up on this? I think we want to add this after all...

[eladb]

Superseded by #4947