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

When there are multiple actions in the queue, only call the "updated" methods for the last action #697

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

adamghill
Copy link
Owner

@adamghill adamghill commented Jun 22, 2024

This is a prototype to solve for #696. It changes the semantics of how the updated methods work, though -- instead of getting called for every action, when there are multiple actions in a queue (for example with deferred), the updated method would only get called once for the last action.

I do wonder:

  1. If adding a new method (instead of using "updated") would be more appropriate, although I can't think of a good name for it.
  2. Could I pass another argument along with the property value which indicates it's the last value in a queue? I can't think of a nice argument name for that either. Something like this arg name feels very messy, but would be "correct": def updated_search(self, search, is_last_or_only_action.
  3. Another option would be add something to the component instance (aka self) which could be used, but this feel like it would expose a lot of internals that probably aren't all that useful to most people. But, theoretically I could add component_request to self, so then component code to do something with the last action would look something like this:
def updated_search(self, search):
    action_queue = self.component_request.action_queue

    if len(action_queue) > 1:
        last_action = action_queue[-1:][0]
            
        if last_action.payload.get("name") == "search" and last_action.payload.get("value") == search:
            print(f"this is the last one action: {search}")

That seems like a lot of boilerplate code for something that should be easier for users of the framework.

@adamghill adamghill self-assigned this Jun 22, 2024
@cro
Copy link

cro commented Jul 16, 2024

Sorry for the lag in responding here. Number 1 above seems to be the cleanest API, avoiding many of the issues you list for the others (like exposing too many internals). In particular it makes it easy for devs to implement both "updated" semantics when they really want to handle every event, as well as just the last event.

After thinking a while I wonder if each_update and last_update would be good names?

@adamghill
Copy link
Owner Author

After thinking a while I wonder if each_update and last_update would be good names?

Makes sense. I think, to prevent changing the existing API, I would keep updated as-is, but add another method for the last update.

  • last_updated_{field}
  • final_updated_{field}
  • finished_updated_{field}
  • finished_{field}

Any of those sound ok? Or something else maybe?

@adamghill
Copy link
Owner Author

I was looking at synonyms for "completed" and ran across "resolved". I think that works and has the right connotation. So, my plan is to add a resolved_search(self, value) method that could be added to components which will only ever fire once with the last value for the field.

@cro
Copy link

cro commented Jul 16, 2024

This sounds logical!

@adamghill adamghill force-pushed the updated-with-multiple-actions branch 2 times, most recently from 5105d12 to ba0e3a1 Compare July 26, 2024 02:56
@adamghill adamghill force-pushed the updated-with-multiple-actions branch from ba0e3a1 to a7237ca Compare July 26, 2024 02:58
@adamghill adamghill merged commit a7237ca into main Jul 26, 2024
5 checks passed
@adamghill adamghill deleted the updated-with-multiple-actions branch July 26, 2024 03:11
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