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

feat(sam): improved TypeScript support #3099

Merged
merged 8 commits into from
Jan 24, 2023
Merged

Conversation

sseidel16
Copy link
Contributor

Problem

TypeScript debugger has a race condition that breaks debugging much of the time. Hard-coded tsconfig also is used which breaks anything more than a very basic typescript lambda.

Solution

Updated toolkit to generate tsconfig that attempts to match project tsconfig as close as possible, code is compiled into a ts build directory, sam deploys from there on an updated handler path (pointing to build dir). This eliminates the debugger race condition, and much more closely resembles a standard project build eliminating all of our errors. It also matches jetbrains approach here.

Code updated:

  • src/shared/sam/activation.ts: allow typescript code to be recognized
  • src/shared/sam/debugger/awsSamDebugger.ts: code template generation after individual runtime configs are made so ts can update handler path
  • src/shared/sam/debugger/typescriptSamDebug.ts: updated build directory process matching jetbrains (most of the updates)
  • src/shared/sam/localLambdaRunner.ts: split getting template path and creating the template to facilitate awsSamDebugger.ts changes
  • src/test/shared/sam/debugger/samDebugConfigProvider.test.ts: update ts tests

Tested with unit tests, and with several typescript projects including large and small lambdas. New code lines are covered with code coverage.

License

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

@sseidel16 sseidel16 requested a review from a team as a code owner January 13, 2023 00:00
src/shared/sam/debugger/typescriptSamDebug.ts Outdated Show resolved Hide resolved
src/shared/sam/debugger/typescriptSamDebug.ts Outdated Show resolved Hide resolved
src/shared/sam/debugger/typescriptSamDebug.ts Outdated Show resolved Hide resolved
src/shared/sam/debugger/typescriptSamDebug.ts Outdated Show resolved Hide resolved
@@ -90,7 +96,7 @@ export async function makeInputTemplate(
.withRuntime(config.lambda!.runtime!)
.withCodeUri(pathutil.normalize(config.invokeTarget.projectRoot))

if (config.lambda?.environmentVariables) {
if (config.lambda?.environmentVariables && Object.entries(config.lambda?.environmentVariables).length) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice improvement.

@JadenSimon
Copy link
Contributor

JadenSimon commented Jan 13, 2023

/runintegrationtests ....

src/shared/sam/debugger/typescriptSamDebug.ts Outdated Show resolved Hide resolved
// Adapted from: https://github.com/aws/aws-toolkit-jetbrains/blob/911c54252d6a4271ee6cacf0ea1023506c4b504a/jetbrains-ultimate/src/software/aws/toolkits/jetbrains/services/lambda/nodejs/NodeJsLambdaBuilder.kt#L60
const defaultTsconfig = {
compilerOptions: {
const tsConfigPath = path.join(config.codeRoot, tsConfigFile)
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be really nice if we could write this to the build directory if possible. Maybe we can use extends instead of directly merging config files?

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.

2 participants