Skip to content

[LiveComponent] improve errors when LiveArg is missing for live action and live listener #1906

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: 2.x
Choose a base branch
from

Conversation

WebMamba
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
Issues Fix
License MIT

This PR improves the error message when a LIveArg is missing on live action or live event. Here is the new error and the previous one.

Capture d’écran 2024-06-10 à 16 43 38 Capture d’écran 2024-06-10 à 16 44 19

@carsonbot carsonbot added Feature New Feature LiveComponent Status: Needs Review Needs to be reviewed labels Jun 10, 2024
@@ -274,6 +275,14 @@ public function onKernelException(ExceptionEvent $event): void
return;
}

if ($event->getThrowable() instanceof RuntimeException) {
if (str_starts_with($event->getThrowable()->getMessage(), 'Could not resolve argument $key')) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use a class/interface here?

@smnandre
Copy link
Member

What will be displayed if the LiveComponent is not tagged with controller_arguments (or does not extends AbstractController) ?

@smnandre
Copy link
Member

Hmm ok ...

It's because of the NotTaggedControllerValueResolver registered only in debug.

We should see this message instead, already better.
Capture d’écran 2024-06-14 à 22 45 01

And, guess what..... all this is related to our LiveArg implementation 🤖

So i suggest we take some time (when we can) to make a good, stable & durable fix for LiveArg

.. and we won't have this problem anymore :à

Copy link
Member

@Kocal Kocal left a comment

Choose a reason for hiding this comment

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

Checking on the exception message is a bit fragile, can't we check on the exception class name or where the extension has been thrown?

Also, tests are missing.

@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature LiveComponent Status: Needs Work Additional work is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants