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

bugfix: x-select options do not refresh #593

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

Conversation

stephancasas
Copy link

@stephancasas stephancasas commented Dec 30, 2022

Slot-provided and attribute-provided options passed to Select (<x-select />) may fail to refresh/redraw if more than one of Select's properties is updated during Livewire's lifecycle.

Consider the following:

StyleEditor.php

class StyleEditor {
  public ?Product $product = null;

  public ?string $selectedOption = null;
  public ?string $optionValue = null;

  public function getStyleOptionsProperty()
  {
    return collect($this->product->styleOptions)
      ->pluck('name')
      ->toArray();
  }

  public function getStyleValueOptionsProperty()
  {
    return blank($this->selectedOption) ?
      [] : collect($this->product->styles)
        ->firstWhere('name', $this->selectedOption)['options'];
  }
}

style-editor.blade.php

@isset($this->product)
  <div>
    <x-select label="Style" :options="$this->styleOptions" wire:model="selectedOption" />
    <x-select label="Setting" 
            :options="$this->styleValueOptions"
            wire:model="optionValue"
            :disabled="blank($this->selectedOption)"/>
  </div>
@endisset

The mutability of and the options available to the second <x-select> are dependent upon the value mutated by the first <x-select> — selectedOption. When selectedOption changes, Livewire will re-compute the value for styleValueOptions, and the available options in the dependent <x-select> should change. Presently this does not occur.

Changes to the markup can be observed in the inspector, but they are not picked-up by the MutationObserver upon which WireUi depends to reflect changes in AlpineJS. To account for this, mutation-invoked operations should be deferred until the end of the current events cycle — after Livewire and AlpineJS has finished manipulating the DOM. In the PR, I have wrapped the syncJsonOptions() and syncSlotOptions() inside of a setTimeout() to do this. Other methods may be feasible or preferred.

Further, it is necessary to include the option label in the key property of each <li> provided to AlpineJS in the x-for used to display the popover options. When a set of options includes options with index and value parity — but not label parity — AlpineJS will not draw the updated label.


I appreciate your insights and thoughts on these considerations, as well as your ongoing work with WireUi. It is a blessing and gift to Laravel and Livewire.

option.html = element.querySelector('[name="wireui.select.slot"]')?.innerHTML

return option
setTimeout(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Have you tried using $nextTick or requestAnimationFrame instead a setTimeout?

Copy link
Author

Choose a reason for hiding this comment

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

That's a good thought — I'll test an implementation in my working version and update here. I continued to experience issues in other contexts even after applying this change and traced it back to a problem in the $wire.entangle() implementation in Livewire's Alpine magic method.

While the order (with respect to MorphDOM's manipulations) still remained relevant, the bigger problem was in Alpine's enumeration of "dot notation" syntax for Livewire properties in contexts where Livewire may add/remove an x-data-assigned element, with a $wire.entangle() property, based on conditional Blade directives. I wound up adding a modified version of the $wire magic to fix this:

Here, I verify that the property is in-fact entangled via x-data in markup

https://gist.github.com/stephancasas/4e37883edfb230fbb5cc8570a4b91007#file-rewire-js-L42-L48

Then, here, I verify that the markup still exists prior to enumerating property values:

https://gist.github.com/stephancasas/4e37883edfb230fbb5cc8570a4b91007#file-rewire-js-L74-L81


My use case was a flyout-style component where I didn't want markup delivered to the DOM (even hidden) until the flyout was actually needed. I then wanted the ability to unmount the flyout's content post-dismissal. In practice, this meant enclosing Livewire-enabled components in a Blade @if()directive, which Livewire would then act on accordingly.

However, after the first flyout dismissal, the <x-select> component would fail entirely — along with any other $wire.entangle()-powered components inside of the @if() directive.

In truth, I'm not happy with using regex to solve the problem, but because I'm not certain that this is an issue worth raising in Alpine/Livewire (especially with v3 on the horizon) it works for now. Further, I realize that the problem could also be solved by swapping @if() for another display/render approach and/or deeper property initialization but, as I was building a component, I wanted the dev experience to feel rapid and a little more seamless as opposed to adding extra consideration.

What do you think?

Copy link
Member

@PH7-Jack PH7-Jack May 1, 2023

Choose a reason for hiding this comment

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

Hey, so sorry for my delay. I think in the next livewire version it will be easier to work on. Livewire and Alpine will be better integrated.
For now, we can use this workaround to fix it.
Is the setTimeout solution better than using $nextTick or requestAnimationFrame?

Copy link
Member

Choose a reason for hiding this comment

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

Your rewire.js is interesting
Do you think it will fix the select problem?

Copy link
Author

Choose a reason for hiding this comment

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

No worries. I've been busy like crazy, and I'm sure you've been occupied with the new WireUI version (super-excited about that, btw).

In different scenarios, I've had mixed results using $nextTick vs. setTimeout. Functionally, they seem to yield the same outcome, so my choice of one vs. the other has come down to what works well in the current Alpine context (i.e. markup vs. js file).

I've been using rewire.js in production apps, and it's solved the x-select issue for me. At one time, I raised the issue in the Livewire discussion group, but it got buried underneath the v3 conversation. Hopefully, there's an apparent fix in v3 that can handle this natively.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants