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

Feature request: (non-experimental-decorator) tracer.captureMethod #3085

Open
1 of 2 tasks
WtfJoke opened this issue Sep 18, 2024 · 3 comments
Open
1 of 2 tasks

Feature request: (non-experimental-decorator) tracer.captureMethod #3085

WtfJoke opened this issue Sep 18, 2024 · 3 comments
Labels
discussing The issue needs to be discussed, elaborated, or refined feature-request This item refers to a feature request for an existing or new utility need-customer-feedback Requires more customers feedback before making or revisiting a decision revisit-in-3-months Blocked issues/PRs that need to be revisited

Comments

@WtfJoke
Copy link

WtfJoke commented Sep 18, 2024

Use case

We use Tracers functionality to capture class methods using decorators as described in the docs.

Typescript 5.0+ has official (non experimental) support for decorators.
However as soon as we remove the tsconfig.json flag experimentalDecorators (or set it to false) we get compile errors:

 error TS1241: Unable to resolve signature of method decorator when called as an expression.
  The runtime will invoke the decorator with 2 arguments, but the decorator expects 3.

34     @tracer.captureMethod()
       ~~~~~~~~~~~~~~~~~~~~~~~

  node_modules/@aws-lambda-powertools/tracer/lib/esm/types/Tracer.d.ts:100:100
    100 type MethodDecorator<T extends AnyClass> = (target: InstanceType<T>, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<AnyClassMethod>) => void;
                                                                                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    An argument for 'descriptor' was not provided.

Solution/User Experience

It would be great if we have support for non experimental decorators so we can remove outdated decorators functionality.

Alternative solutions

No response

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

@WtfJoke WtfJoke added feature-request This item refers to a feature request for an existing or new utility triage This item has not been triaged by a maintainer, please wait labels Sep 18, 2024
@dreamorosi
Copy link
Contributor

Hi @WtfJoke, thanks for opening the issue.

This has been on my radar for a while but you're the first one to request it.

Technically speaking the change is doable, but it would be a breaking change we can't do it without a major version bump. Besides the change itself and the version bump, releasing a new major version would require us a significant amount of work in documentation, outreach, and support that we are not yet ready to take on - at least not until the end of the year.

Something that I would like to explore is whether we can support both versions of the decorator at the same time, possibly using the typeVersions field and some runtime checks on the shape of the arguments.

From a first exploration that I did when TS switched implementation, for what this library is concerned only modify the signature of the decorator factory & function. We'll have to see if we can support both at runtime and selectively chose the types.

For future readers, if you also want this feature please leave 👍 on the original comment and/or leave a comment with your use case. Having this type of influence will help us reassess the priority as needed.

@dreamorosi dreamorosi added need-customer-feedback Requires more customers feedback before making or revisiting a decision discussing The issue needs to be discussed, elaborated, or refined revisit-in-3-months Blocked issues/PRs that need to be revisited and removed triage This item has not been triaged by a maintainer, please wait labels Sep 18, 2024
@WtfJoke
Copy link
Author

WtfJoke commented Sep 19, 2024

releasing a new major version would require us a significant amount of work in documentation, outreach, and support that we are not yet ready to take on - at least not until the end of the year.

Yeah I was hoping/assuming that there is no need for a major version change in implementing this feature. Instead maybe introducing a new method and deprecate the old one (and remove the deprecated one in the next major release).

I can understand that the next major version will be not around any soon :D.

Something that I would like to explore is whether we can support both versions of the decorator at the same time, possibly using the typeVersions field and some runtime checks on the shape of the arguments.

From a first exploration that I did when TS switched implementation, for what this library is concerned only modify the signature of the decorator factory & function. We'll have to see if we can support both at runtime and selectively chose the types.

That sounds promising and would be great (and match my hopes from above 😄 )!

Thanks for replying so fast and share your insights @dreamorosi

@dreamorosi
Copy link
Contributor

No problem, also added this to the v3 feature list discussion #2560

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussing The issue needs to be discussed, elaborated, or refined feature-request This item refers to a feature request for an existing or new utility need-customer-feedback Requires more customers feedback before making or revisiting a decision revisit-in-3-months Blocked issues/PRs that need to be revisited
Projects
Development

No branches or pull requests

2 participants