Skip to content

fix(repeater): bubble afterStateUpdated to parents #16093

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 1 commit into
base: 3.x
Choose a base branch
from

Conversation

Domianidze
Copy link
Contributor

Description

This PR addresses an issue where afterStateUpdated callbacks on parent components were not being triggered after running repeater-related actions such as add, delete, reorder, etc.

In these cases, the component would call afterStateUpdated on itself at the end of the action, but unlike normal state updates, the update did not bubble up to its parent components. This meant any logic relying on afterStateUpdated at the parent level would silently fail to run.

To fix this, a new helper was introduced that first checks if the component is live, and then recursively walks up the component tree, calling afterStateUpdated on each parent. This mirrors the behavior of actual state updates, ensuring that all necessary callbacks are triggered.

This approach has worked well in our testing and reflects the expected behavior of how state updates should propagate. Let me know if there’s a more appropriate way to handle this logic, or if there are any edge cases we might’ve missed.

Visual changes

No visual changes.

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@github-project-automation github-project-automation bot moved this to Todo in Roadmap Apr 21, 2025
@danharrin danharrin added this to the v3 milestone Apr 21, 2025
@danharrin danharrin added the bug Something isn't working label Apr 21, 2025
@danharrin
Copy link
Member

When you mention parent components, can you provide an example of some code that is currently broken? I'm not even sure that running the hook on parents is desired behaviour but I would like to hear what you are using this for first?

@Domianidze
Copy link
Contributor Author

Domianidze commented Apr 26, 2025

For example, in our case we have a Section component that contains various fields, some of which are Repeaters. On the side, we have a live preview that should update anytime any field changes. On the section we have an afterStateUpdated function set that refreshes the preview.

Example:

Section::make()
    ->schema([
        TextInput::make('heading'),
        Repeater::make('blocks')
            ->schema([
                TextInput::make('image'),
            ]),
    ])
    ->afterStateUpdated(fn () => $this->updatePreview())
    ->live(),

Normally, if a user edits the heading field, the afterStateUpdated hook runs, and the preview refreshes.

Problem:

If a user adds, deletes, reorders or does any of the repeater actions inside the blocks Repeater, the Repeater would call its own afterStateUpdated, but the Section’s afterStateUpdated doesn’t get triggered. As a result, the preview doesn’t update.

This is why we made the change to bubble afterStateUpdated up through the parent components to match how normal state updates behave.

@danharrin
Copy link
Member

Does the section have a statePath() defined or anything in your example?

@Domianidze
Copy link
Contributor Author

Domianidze commented Apr 27, 2025

Only thing relevant it has is live(), which is what causes the normal field changes to bubble up the afterStateUpdated to the Section. That’s why I’m checking if the Repeater is live before bubbling it up to mirror the normal state updates.

(Sorry, I forgot to include that in the example.)

@danharrin
Copy link
Member

Honestly that is super weird behaviour, I never knew it did that. So I will need to look into the underlying cause and decide whether or not that is desired behaviour. Sorry, that will result in a delay to this PR.

@Domianidze
Copy link
Contributor Author

No worries, I’ll be here to help if I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs more info
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants