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: update TS and JS init templates to consistently use ES6 imports #7356

Merged
merged 2 commits into from
Apr 17, 2020

Conversation

ghost
Copy link

@ghost ghost commented Apr 15, 2020

Commit Message

chore: update TS and JS init templates to consistently use ES6 imports

End Commit Message

I've recently updated the TypeScript code snippets in the AWS CDK Developer Guide to use ES6-style imports so they can be auto-translated to JavaScript using Babel. (Babel won't translate the older style require() imports.)

Since the Guide snippets are (at least in theory) what users would see when working in a project they created using 'cdk init' and a template, let's update the templates to use ES6 imports.

There are a fair number of old-style imports in the various READMEs as well; I'll update those in a separate PR.


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

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Apr 15, 2020
@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: aca23a2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Apr 17, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: a94652d
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Apr 17, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 6d913fd into master Apr 17, 2020
@mergify mergify bot deleted the jerry-aws-template-es6-imports branch April 17, 2020 22:24
@ghost
Copy link
Author

ghost commented Apr 21, 2020

This actually broke the JS init templates since Node still doesn't actually support ES6 imports. I experimented with using the esm module, which allowed the JS app to run, but Jest was still broken. The Jest issue also affects the TS init templates.

So I'm going to revert most of this PR. Unfortunately, these changes made it into a release.

@shivlaks
Copy link
Contributor

@jerry-aws - yes I can confirm we have a regression with JS templates. What's the Jest issue? I'm able to init and run my templates fine in TS

@ghost
Copy link
Author

ghost commented Apr 21, 2020

@shivlaks Looks like the Jest issue doesn't affect TS as I thought; the tests are .ts files so tsc will convert the imports to what Jest wants.

Still, no loss to revert the whole PR.

mergify bot pushed a commit that referenced this pull request Apr 21, 2020
This was a result of switching the templates to use ES6 style imports in #7356, which Node does not support yet and not with our minimum of `10.12.0`.
shivlaks added a commit that referenced this pull request Apr 22, 2020
This was a result of switching the templates to use ES6 style imports in #7356, which Node does not support yet and not with our minimum of `10.12.0`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants