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.

Sign up for GitHub

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

Jump to bottom

[TwigComponent] Improve exception message when component not found #1031

Merged
merged 1 commit into from
Aug 15, 2023
Merged

[TwigComponent] Improve exception message when component not found #1031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion src/TwigComponent/src/ComponentFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,32 @@ private function isAnonymousComponent(string $name): bool
*/
private function throwUnknownComponentException(string $name): void
{
throw new \InvalidArgumentException(sprintf('Unknown component "%s". The registered components are: %s. And no matching anonymous component template was found', $name, implode(', ', array_keys($this->config))));
$message = sprintf('Unknown component "%s".', $name);
$lowerName = strtolower($name);
$nameLength = \strlen($lowerName);
$alternatives = [];

foreach (array_keys($this->config) as $type) {
$lowerType = strtolower($type);
$lev = levenshtein($lowerName, $lowerType);

if ($lev <= $nameLength / 3 || str_contains($lowerType, $lowerName)) {
$alternatives[] = $type;
}
Comment on lines +227 to +231
Copy link
Contributor

Choose a reason for hiding this comment

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

What about low cost check first instead of doing both?

Suggested change
$lev = levenshtein($lowerName, $lowerType);
if ($lev <= $nameLength / 3 || str_contains($lowerType, $lowerName)) {
$alternatives[] = $type;
}
if (str_contains($lowerType, $lowerName)) {
$alternatives[] = $type;
} elseif (levenshtein($lowerName, $lowerType) <= $nameLength / 3) {
$alternatives[] = $type;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you going to get this exception all the time? 🙂

}

if ($alternatives) {
if (1 === \count($alternatives)) {
$message .= ' Did you mean this: "';
} else {
$message .= ' Did you mean one of these: "';
}

$message .= implode('", "', $alternatives).'"?';
} else {
$message .= ' And no matching anonymous component template was found.';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice! Just tried it out, it's super useful! Do you think we can apply the Levenshtein to look also for AnonymousComponent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 probably all available twig templates then should be iterated :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As I'm not using anonymous components, then I'll leave it for you as that was your feature :)

}

throw new \InvalidArgumentException($message);
}
}
17 changes: 12 additions & 5 deletions src/TwigComponent/tests/Integration/ComponentFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,24 @@ public function testCanGetMetadataForSameComponentWithDifferentName(): void
public function testCannotGetConfigByNameForNonRegisteredComponent(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessageMatches('/^Unknown component "invalid"\. The registered components are:.* component_a/');
$this->expectExceptionMessage('Unknown component "tabl". Did you mean this: "table"?');

$this->factory()->metadataFor('invalid');
$this->factory()->metadataFor('tabl');
}

public function testCannotGetInvalidComponent(): void
/**
* @testWith ["tabl", "Unknown component \"tabl\". Did you mean this: \"table\"?"]
* ["Basic", "Unknown component \"Basic\". Did you mean this: \"BasicComponent\"?"]
* ["basic", "Unknown component \"basic\". Did you mean this: \"BasicComponent\"?"]
* ["with", "Unknown component \"with\". Did you mean one of these: \"with_attributes\", \"with_exposed_variables\", \"WithSlots\"?"]
* ["anonAnon", "Unknown component \"anonAnon\". And no matching anonymous component template was found."]
*/
public function testCannotGetInvalidComponent(string $name, string $expectedExceptionMessage): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessageMatches('/^Unknown component "invalid"\. The registered components are:.* component_a/');
$this->expectExceptionMessage($expectedExceptionMessage);

$this->factory()->get('invalid');
$this->factory()->get($name);
}

public function testInputPropsStoredOnMountedComponent(): void
Expand Down
Loading