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

Javascript error occurs after an action that displays a modal is run and the modal closes. #3665

Closed
jojostx opened this issue Aug 25, 2022 · 29 comments · Fixed by #3810
Closed
Labels
bug Something isn't working confirmed

Comments

@jojostx
Copy link
Contributor

jojostx commented Aug 25, 2022

Package

filament/filament

Package Version

^2.13

Laravel Version

v9.11.0

Livewire Version

v2.5

PHP Version

PHP 8.0

Problem description

After an action that requires confirmation or displays a modal is run, and the modal closes, a javascript error occurs:

Alpine: Cannot reference "$wire" outside a Livewire component.
Uncaught TypeError: wireEl is null

Expected behavior

The action completes and the modal closes without a Javascript error occurring

Steps to reproduce

  1. Login to Filament admin panel Demo online
  2. visit the edit page for any record/row in the brands page
  3. eg: https://demo.filamentphp.com/shop/brands/11/edit?activeRelationManager=0
  4. delete a related model on the relationship table eg: products relations table.
  5. open browser console
  • before confirming action:

Screenshot (33)

  • after confirming action:

Screenshot (32)

Reproduction repository

https://github.com/jojostx/filament-demo

Relevant log output

No response

@jojostx jojostx added bug Something isn't working unconfirmed labels Aug 25, 2022
@github-actions
Copy link

Hey @jojostx! We're sorry to hear that you've hit this issue. 💛

However, it looks like you forgot to fill in the reproduction repository URL. Can you edit your original post and then we'll look at your issue?

We need a public Git repository which contains a Laravel app with the minimal amount of Filament code to reproduce the problem. That would allow us to download it and review your bug much easier, so it can be fixed quicker. Please make sure to include a database seeder with everything we need to set the app up quickly.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 25, 2022
@github-actions
Copy link

Thank you for providing reproduction steps! Reopening the issue now.

@github-actions github-actions bot reopened this Aug 25, 2022
@byjujohn
Copy link

@jojostx Seems like if you edit a record this occurs too.

image

This is happening on a project I am working on as well, in a similar case where a popup modal is shown on the RelationsManager.

Thanks

@jojostx jojostx closed this as completed Aug 26, 2022
@jojostx jojostx reopened this Aug 26, 2022
@jojostx
Copy link
Contributor Author

jojostx commented Aug 26, 2022

@byjujohn I suspect it has to do with the way the modal is closed after an action is dispatched,

@jojostx jojostx changed the title Javascript error occurs after an action that requires confirmation is run and modal closes. Javascript error occurs after an action that displays a modal is run and the modal closes. Aug 26, 2022
@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

This looks like a scoping issue or a bug in Alpine.

Apparently, $wire is not defined inside $watch as it is not declared on this.

This fixes the issue and I'm happy to do a PR for this, but something doesn't feel right about it. Any thoughts @zepfietje @danharrin

this.wire = $wire.__instance;
$watch('isOpen', () => {
      if (isOpen) {
          return
      }
      this.wire.mountedTableAction = null
})

@zepfietje
Copy link
Member

I've had a similar issue with $wire being undefined when closing notifications. That's why I updated that to Livewire.emit().

However, for this case that wouldn't work. Your fix may be kind of a dirty fix, but if it works... Not sure if this issue got introduced just recently and is a regression, or really a bug with Alpine?

@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

This has to be something in Alpine. $wire is available outside of $watch.

@zepfietje
Copy link
Member

@danharrin mentioned he had a potential fix ready locally. Let's see what he'll PR.

@danharrin
Copy link
Member

@awcodes I submitted PR #3810 which completely removes that code. It was originally added to allow people to customize their action forms based on Create or Edit, but it never worked properly as some actions use mountedAction and some use mountedTableAction. I don't see any reason for people to manually check the $this->mountedAction when we have $context now.

@danharrin
Copy link
Member

But potentially it is a breaking change if we remove it...

@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

So, we're good on this?

@danharrin
Copy link
Member

Is it a Livewire bug that $wire doesnt work inside watchers?

@danharrin
Copy link
Member

I'm sure this worked previously

@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

Hard to say. The error is getting thrown by SupportAlpine.js which is in livewire, but the fact that $wire is always null inside $watch could be an Alpine bug.

Screen Shot 2022-09-01 at 3 14 17 PM

@danharrin
Copy link
Member

Does this.wire = $wire in your fix not work? Does setting a property on the instance really reflect that change in the backend using an AJAX request?

@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

this.wire = $wire works, it stops the error from happing. Just a really gross way of doing it, IMO.

but it does have to bet $wire.__instance, other wise it throughs a 500.

@danharrin
Copy link
Member

But does it make an AJAX request with your fix?

@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

I'm seeing the same AJAX requests with or without the fix

@danharrin
Copy link
Member

weird... so the console error is there for nothing?

@awcodes
Copy link
Contributor

awcodes commented Sep 1, 2022

Looks like it. LOL

Maybe it's some weird race condition.

@zepfietje
Copy link
Member

Looks like it. LOL

Maybe it's some weird race condition.

Had the same with notifications. Worked fine despite the error.

@danharrin
Copy link
Member

And did you fix that?

@zepfietje
Copy link
Member

And did you fix that?

#3665 (comment)

I first solved that case by passing $wire into the JS file instead of accessing this.$wire. That seemed to fix it. Later converted $wire.emit() to Livewire.emit() which I found cleaner. Unfortunately, that won't work for this exact issue, as you need to call a method on the right Livewire instance.

@danharrin
Copy link
Member

Alright. I am going to update my PR to @awcodes's suggestion

@zepfietje
Copy link
Member

Seems a good solution for now.

@danharrin
Copy link
Member

Turns out that setting __instance.property doesnt make an AJAX request, so the solution doesn't work.

I swapped it out for __instance.set('property', null) which works fine

@bokoch
Copy link

bokoch commented Sep 18, 2022

@danharrin Any news regarding this issue? I see it still reproduced and looks like fix was reverted.
image

Also found that if modal have repeater it is not working, collapsing/expanding, sorting of repeater items

@danharrin
Copy link
Member

Hey, the fix was not reverted, we just didn't need any of the code that handled closing modals any more. I can't reproduce the problem myself any more? Please submit a new issue with a reproduction repository.

@bokoch
Copy link

bokoch commented Sep 19, 2022

Ok, thanks. I think similar issue already created here #3906

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working confirmed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants