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

<x-dynamic-component /> support? #707

Closed
stevenmaguire opened this issue Jun 14, 2024 · 7 comments · Fixed by #708
Closed

<x-dynamic-component /> support? #707

stevenmaguire opened this issue Jun 14, 2024 · 7 comments · Fixed by #708

Comments

@stevenmaguire
Copy link

Is the Laravel documented support for Dynamic Components is expected to work in Jigsaw?

An exception is raised when trying it out and it's unclear if it is local to the project or broadly unsupported in Jigsaw.

$componentType = 'something-kebab-case';

<x-dynamic-component :component="$componentType" />
   Spatie\LaravelIgnition\Exceptions\ViewException 
  Target [Illuminate\Contracts\Foundation\Application] is not instantiable.

In this example, a component is confirmed as working when expressed explicitly, so it is not a missing component issue.

This works just fine.

<x-something-kebab-case />

The error feels architectural.

I also noticed there is some reference to dynamic component support in the Jigsaw providers, but there are also some code comments and TODOs.

private function registerBladeCompiler(): void
{
$this->app->singleton('blade.compiler', function (Container $app) {
// TODO $app['config']['view.compiled']
return tap(new BladeCompiler($app['files'], $app->cachePath()), function ($blade) {
$blade->component('dynamic-component', DynamicComponent::class);
});
});
// v1 binding is 'bladeCompiler'
$this->app->alias('blade.compiler', 'bladeCompiler');
}

Is this expected to be supported and unfinished?

@stevenmaguire
Copy link
Author

Digging into this further, there is definitely something incomplete here in Jigsaw. I'll try to explain what that might be and I think that Tighten may want to decide if this functionality is actually meant to be supported.

tl;dr - it seems to be incomplete. hopefully my observations will help to narrow the scope of what can be done to add support, if that is important.


The main seam where this support is falling apart is in the compiler method that is attempting to guess the component name, view path, and/or class name.

Illuminate\View\Compilers\ComponentTagCompiler::componentClass(string $component);
CleanShot 2024-06-14 at 15 39 46@2x

The exception initially reported was cause by a make call on the container to resolve a class that is bound to the Illuminate\Contracts\Foundation\Application interface; the method responsible for that is Illuminate\View\Compilers\ComponentTagCompiler::guessClassName.

https://github.com/laravel/framework/blob/6c0561caeb8265b7d17cabeea0ec7d4c5cca28aa/src/Illuminate/View/Compilers/ComponentTagCompiler.php#L409-L418

I added the following to my bootstrap.php file to get past the resolution issue.

$events->beforeBuild([
    fn ($jigsaw) => $jigsaw->app->bind(\Illuminate\Contracts\Foundation\Application::class, fn () => $jigsaw->app)
]);

$jigsaw->app, which is an instance of TightenCo\Jigsaw\Container does not currently implement that interface (perhaps that could|would be a good idea?) and that doesn't matter. What does matter, and for this very specific use case, is that it furnishes a getNamespace method that returns a string. The returned string is expected to be used to fill out a Laravel-specific namespace to find a component - this will always fail. So, it can return an empty string.

That is not going to produce a value for $class so the logic will move on to guessAnonymousComponentUsingNamespaces and guessAnonymousComponentUsingPaths.

guessAnonymousComponentUsingNamespaces is likely going to always fail, even if you attempt to register the local \Components namespace because there is no actual namespace. guessAnonymousComponentUsingPaths feels like the right place to focus effort to resolve a good component name.

Although, at the moment, even attempting to register a path in blade.php, there is yet another missing container binding for views, just like the Application interface binding mentioned above.

$events->beforeBuild([
    fn ($jigsaw) => $jigsaw->app->bind(\Illuminate\Contracts\View\Factory::class, fn () => $jigsaw->app->make('view'))
]);
$bladeCompiler->anonymousComponentPath(__DIR__.'/source/_components');

This still fails due to some config issue, also with TODO code comments in vendor/tightenco/jigsaw/src/Providers/ViewServiceProvider.php:57.

CleanShot 2024-06-14 at 15 49 55@2x

@stevenmaguire
Copy link
Author

Sorry for the mention @damiani but I am curious if the Tighten team has an opinion on this? Is this an incomplete expected feature or something that should not be expected?

@damiani
Copy link
Contributor

damiani commented Jun 25, 2024

@stevenmaguire Thanks for the reminder ... I'll take a look and see what we'll need to do to support this; appreciate your detailed analysis of the code paths involved. More shortly!

@stevenmaguire
Copy link
Author

@damiani thanks for the prompt response. Sorry again to escalate - it seems like an oversight one way or another.

@bakerkretzmar
Copy link
Collaborator

bakerkretzmar commented Jun 25, 2024

@stevenmaguire what's the folder structure of the project you're trying this in? Where are your something-kebab-case component and the view you're trying to use it in? Does it work if you use components.something-kebab-case as the name?

That specific TODO I'm pretty sure isn't related to the dynamic components, it was about using that container binding instead of $app->cachePath().

It looks like I added this in #662 as part of a much larger refactor, but I'll keep staring at it and see if I remember anything.

@bakerkretzmar
Copy link
Collaborator

Actually we're using our own ComponentTagCompiler to do some custom path/namespace resolution stuff, I wonder if we just need to use our own DynamicComponent too so that we can force it to use our compiler instead of Laravel's.

@stevenmaguire
Copy link
Author

It looks like I added this in #662 as part of a much larger refactor, but I'll keep staring at it and see if I remember anything.

Actually we're using our own ComponentTagCompiler to do some custom path/namespace resolution stuff, I wonder if we just need to use our own DynamicComponent too so that we can force it to use our compiler instead of Laravel's.

These two statements feel more relevant than the first question asking about the folder structure. I am highly confident it is not simply a case of pathing and/or directory issues. After I stepped through it all, I am firmly in the camp of "it feels like an architectural or plumbing issue." Using your own compiler would likely resolve it and force you to touch each of the seams I identified in my second comment here.

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

Successfully merging a pull request may close this issue.

3 participants