Skip to content
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

chore: upgrade to @types/node 10.x #894

Merged
merged 2 commits into from
Oct 18, 2019
Merged

chore: upgrade to @types/node 10.x #894

merged 2 commits into from
Oct 18, 2019

Conversation

RomainMuller
Copy link
Contributor

Actually upgrade the node engine support to 10.x tree, as has
been declared in the package.json files for a month now.

Additionally, standardized the compilerOptions in the tsconfig.json files
that are not generated by jsii, in order to use ES2018 standard library,
enable strict mode (this includes strictPropertyInitialization, which was not
enabled previously and required a few code changes in jsii-pacmak that were
made ad-minima).

The library clause of tsconfig.json files that are generated by jsii has not
been changed yet. This means you need to use node >= 10.x in order to use
jsii, but you cannot use it to build code that depend on APIs that are not yet
available in node 8. This behavior will be updated in a second phase.

Fixes #794


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

Actually upgrade the `node` engine support to 10.x tree, as has
been declared in the `package.json` files for a month now.

Additionally, standardized the `compilerOptions` in the `tsconfig.json` files
that are *not* generated by `jsii`, in order to use `ES2018` standard library,
enable `strict` mode (this includes `strictPropertyInitialization`, which was not
enabled previously and required a few code changes in `jsii-pacmak` that were
made ad-minima).

The library clause of  `tsconfig.json` files that are generated by `jsii` has not
been changed yet. This means you need to use `node >= 10.x` in order to use
`jsii`, but you cannot use it to build code that depend on APIs that are not yet
available in `node 8`. This behavior will be updated in a second phase.

Related: #794
@RomainMuller RomainMuller requested review from bmaizels and a team as code owners October 17, 2019 13:20
@RomainMuller RomainMuller removed the request for review from bmaizels October 17, 2019 13:20
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

Copy link
Contributor

@nija-at nija-at left a comment

Choose a reason for hiding this comment

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

Have you run a sanity check against a few CDK modules to ensure that all of the JSII libraries are consumed and function correctly? Including libraries that consume jsii-reflect, and the like.

@RomainMuller
Copy link
Contributor Author

@nija-at so I have ran a full build of aws-cdk with this version of jsii and then proceeded to run all tests using the result in a Docker container running the node:8 (containing NODE_VERSION=8.16.2) image (asserting that no code was making incompatible calls yet). This uncovered no issue.

@mergify
Copy link
Contributor

mergify bot commented Oct 18, 2019

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Oct 18, 2019
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

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

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

@mergify mergify bot merged commit e8c7b12 into master Oct 18, 2019
@mergify mergify bot deleted the rmuller/upgrade-node branch October 18, 2019 16:04
@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Oct 18, 2019
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.

Upgrade node requirement to >= 10
3 participants