Skip to content

fix(event-handler): fix decorated scope in appsync events #3974

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

dreamorosi
Copy link
Contributor

Summary

Changes

Please provide a summary of what's being changed

This PR improves the decorator logic of the AppSyncEventsResolver by fixing a fundamental issue that prevented it from being useful.

Prior to this fix the decorators would consistently cause the decorated methods to become unbound and lose the reference to this, preventing customers to access other class methods or properties from within the decorated methods.

This was happening because unlike other decorator implementations in the codebase, the one in this area of the codebase works in two steps:

  1. at init time the decorator saves a reference of the decorated method for later usage
  2. at runtime the app.resolve method can call a decorated method depending on routing decisions

By the time the reference is called, the initial scope is lost causing the issue. At the same time, the this scope is not yet available at init time - this is because decorators are evaluated before class init - so it's not possible to save a reference or bind this along with the the decorated method.

Because of this, we need to get around this decorator design aspect.

Specifically, at runtime (step 2) - which is when app.resolve() is called - we introspect the class methods, find their definition, and when we encounter the .resolve keyword (which represents the method), we save a reference to this for later use. This reference is then used and passed to the decorated method to apply the correct this value.

While this is relatively unorthodox, as long as we find one instance of this that belongs to the class to which the decorated method belongs it should be fine.

Please add the issue number below, if no issue is present the PR might get blocked and not be reviewed

Issue number: fixes #3973


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@dreamorosi dreamorosi self-assigned this May 24, 2025
@boring-cyborg boring-cyborg bot added event-handler This item relates to the Event Handler Utility tests PRs that add or change tests labels May 24, 2025
@pull-request-size pull-request-size bot added the size/L PRs between 100-499 LOC label May 24, 2025
@github-actions github-actions bot added the bug Something isn't working label May 24, 2025
Copy link

@dreamorosi dreamorosi added the do-not-merge This item should not be merged label May 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working do-not-merge This item should not be merged event-handler This item relates to the Event Handler Utility size/L PRs between 100-499 LOC tests PRs that add or change tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: AppSyncEventsResolver decorators break scope
1 participant