Skip to content

feat(lambda-nodejs): typescript emitDecoratorMetadata support#16543

Merged
mergify[bot] merged 24 commits intoaws:masterfrom
hassanazharkhan:feat/pre-compilation
Oct 26, 2021
Merged

feat(lambda-nodejs): typescript emitDecoratorMetadata support#16543
mergify[bot] merged 24 commits intoaws:masterfrom
hassanazharkhan:feat/pre-compilation

Conversation

@hassanazharkhan
Copy link
Copy Markdown
Contributor

@hassanazharkhan hassanazharkhan commented Sep 19, 2021

closes #13767

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

@gitpod-io
Copy link
Copy Markdown

gitpod-io bot commented Sep 19, 2021

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Sep 19, 2021

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

@hassanazharkhan hassanazharkhan changed the title feat(lambda-nodejs) experimental decorators support feat(aws-lambda-nodejs) experimental decorators support Sep 19, 2021
@hassanazharkhan hassanazharkhan changed the title feat(aws-lambda-nodejs) experimental decorators support feat(aws-lambda-nodejs): experimental decorators support Sep 19, 2021
@matthias-pichler matthias-pichler mentioned this pull request Sep 21, 2021
Closed
@skinny85 skinny85 requested a review from nija-at September 21, 2021 19:18
@hassanazharkhan
Copy link
Copy Markdown
Contributor Author

hassanazharkhan commented Sep 22, 2021

@nija-at Could you please spare sometime to review this?

@jogold
Copy link
Copy Markdown
Contributor

jogold commented Sep 24, 2021

Hey @hassanazharkhan, from the esbuild doc I see there is some support for experimental decorators already? https://esbuild.github.io/content-types/#typescript

Is it different?

@hassanazharkhan
Copy link
Copy Markdown
Contributor Author

hassanazharkhan commented Sep 24, 2021 •
edited
Loading

Hey @hassanazharkhan, from the esbuild doc I see there is some support for experimental decorators already? https://esbuild.github.io/content-types/#typescript

Is it different?

Hi @jogold, Thank you for your reply, it is indeed different, I would suggest have a look on the following issue evanw/esbuild#1597 for clarification.

Yes, esbuild does support decorators but doesn't support emitDecoratorsMetadata so in this I tried to cover that missing part by transpling ts files using tsc and then forward the transpiled file to esbuild for bundling.

@hassanazharkhan hassanazharkhan changed the title feat(aws-lambda-nodejs): experimental decorators support feat(aws-lambda-nodejs): TypeScript's emitDecoratorMetadata support Sep 26, 2021
@nija-at
Copy link
Copy Markdown
Contributor

nija-at commented Sep 27, 2021

@jogold - Thanks for jumping in.
I have little exposure to eslint, would appreciate if you can handle this pull request on my behalf.

jogold
jogold suggested changes Sep 29, 2021
View reviewed changes
@hassanazharkhan
Copy link
Copy Markdown
Contributor Author

hassanazharkhan commented Oct 3, 2021

@jogold addressed your comments, would appreciate another pass!

@nija-at nija-at changed the title feat(aws-lambda-nodejs): TypeScript's emitDecoratorMetadata support feat(lambda-nodejs): typescript emitDecoratorMetadata support Oct 4, 2021
jogold
jogold reviewed Oct 5, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jogold jogold left a comment

Choose a reason for hiding this comment

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

Question: what workaround are you currently using to use emitDecoratorMetadata in your projects? Are you compiling your whole project with a single tsc run and then pointing the entry to the .js file? Do you have different tsconfig.json files for different handlers and have to run tsc multiple times?

@hassanazharkhan
Copy link
Copy Markdown
Contributor Author

hassanazharkhan commented Oct 5, 2021

Question: what workaround are you currently using to use emitDecoratorMetadata in your projects? Are you compiling your whole project with a single tsc run and then pointing the entry to the .js file? Do you have different tsconfig.json files for different handlers and have to run tsc multiple times?

As discussed offline, I'm using single tsc command to bundle all of my code and then pointing the entry to compiled .js file outputted from tsc.

@hassanazharkhan
Copy link
Copy Markdown
Contributor Author

hassanazharkhan commented Oct 6, 2021

@jogold would appreciate another pass!

jogold
jogold reviewed Oct 8, 2021
View reviewed changes
jogold
jogold reviewed Oct 14, 2021
View reviewed changes
jogold
jogold reviewed Oct 16, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@jogold jogold left a comment

Choose a reason for hiding this comment

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

@nija-at I think this is now ready for your review, can you take a look? Thx.

README and naming to be discussed maybe.

nija-at
nija-at previously requested changes Oct 25, 2021
View reviewed changes
Copy link
Copy Markdown
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.

Thanks for the PR @hassanazharkhan and the review @jogold.

Just some comments from me around docs.

@mergify mergify bot dismissed nija-at’s stale review October 25, 2021 21:40

Pull request has been modified.

@hassanazharkhan
Copy link
Copy Markdown
Contributor Author

hassanazharkhan commented Oct 25, 2021

Thanks, @nija-at for the review, would appreciate another pass!

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Oct 26, 2021

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
Copy Markdown
Collaborator

aws-cdk-automation commented Oct 26, 2021

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: a40dc46
  • 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 55d3c50 into aws:master Oct 26, 2021
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Oct 26, 2021

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).

@modul modul mentioned this pull request Dec 14, 2021
Closed
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@hassanazharkhan @TikiTDO
feat(lambda-nodejs): typescript emitDecoratorMetadata support (aws#16543
05d0c17 
)

closes [aws#13767](aws#13767)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@hassanazharkhan hassanazharkhan mentioned this pull request Aug 23, 2022
Closed
@keikumata keikumata mentioned this pull request Nov 18, 2022
Open
@giseburt giseburt mentioned this pull request Feb 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@jogold jogold jogold requested changes

@nija-at nija-at nija-at approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@nija-at nija-at

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

(aws-lambda-nodejs): esbuild does not support emitting typescript decorator metadata

4 participants

@hassanazharkhan @jogold @nija-at @aws-cdk-automation