ux icon indicating copy to clipboard operation
ux copied to clipboard

[TwigComponent] AnonymousComponentRegistry to share anonymous component in your bundles

Open WebMamba opened this issue 6 months ago • 8 comments

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

Let's get crazy! And let the community share anonymous components through their bundles! This PR introduces a new AnonymousComponentRegistry interface that lets your share the anonymous component inside your bundle

WebMamba avatar Feb 04 '24 11:02 WebMamba

About this PR

I have not much time, but please read attentively what i wrote in the other topic i shared on slack about this topic i've been working on since some months now.

There must be a unique way to call anonmous components, without what the bundle developpers won't be able to nest together components. There can be a shorten/alias/namespace/xxx one.. but there 100% need one "unique full official name" for a component.

From a DX pov, but a security pov too, there cannot be component that automatically replaces userland ones, or other bundle ones.


Personal disgression

Finally guys, to be 100% transparent, i'm starting to be a bit frustrated to think about things, point where the difficulties can be, what need to be solve, debug things... and then all that beeing ignored as if it did not matter... .... then finding myself doing a lot of the support concerning issues, debugging things, etc...

Obviously i'm overstating things there, but it's been weeks i'm asking about the way we want to distinct userland components and bundle ones, ... and i have multiple pocs ready, depending on what we decide.

And i'm again in the situation i have to take some grumpy role, explaining once again why some thing can/cannot work.

I have probably a lot to improve in the way i express myself (and i'm 100% open to advices on this -- on everything in fact). I'm just starting to feel a bit tired of working in vain, studiying the internal of Twig, of UX packages, thinking the usages, the limitations, anticipating major security/performances/maintenance issues, and sometimes feeling a bit alone doing all that.

smnandre avatar Feb 05 '24 19:02 smnandre

Hey @smnandre -

Really sorry you’re feeling that your efforts, info gathering and POCs are being ignored. I think that’s a fair criticism. Personally, due to time constraints, I have a hard time following all the conversations- especially longer, more technical conversations (and you often do deep research that involves full information). My guess is that this is probably true of others. From experience, I know that PRs (even flawed) tend to illicit more conversation than technical docs. For better or worse, we tend to react more to built things.

So, I hope that helps explain a bit at least. Your efforts around here are HUGE and appreciated. In fact, I think you may be sometimes thinking 3 steps ahead of everyone else. You also, I clearly know, do a lot of the unsung work - support, CI tweaks etc - that help make the project work. I’m not sure what workflow is best for discussing these more complex items, but I’m open to ideas. And, again, from experience, PRs are a powerful tool - even to show how an implementation almost works… but has some flaws. And you can always ping me - but you already know that ;)

weaverryan avatar Feb 05 '24 20:02 weaverryan

Re: this PR.

I also feel that users should control the naming of components - not the bundle itself. I’m also not convinced that anonymous components need to be allowed for 3rd party packages.

What I think makes sense is a way to “import” a component from a package (and name it when you do) or import an entire namespace (allowing the bundle to provide names), but with an optional namespace prefix. FWIW, all of this would match how Blade components handle things.

Cheers!

weaverryan avatar Feb 05 '24 20:02 weaverryan

I’m also not convinced that anonymous components need to be allowed for 3rd party packages.

I'm now thinking the same thing. At least until a time there is a significant performance boost between simple class-based components and anon components.

As it stands, 3rd party bundles have an avenue to auto-register there own class-based components. While not perfect, it works.

kbond avatar Feb 05 '24 20:02 kbond

I'm thinking that, while bundles can register their own components, we should discourage that (we can't prevent it) and instead offer a nice "import component" option and encourage 3rd party components to be written in a way that makes them NOT automatically registered, but ready to be imported.

Having some YAML config would be the easiest way. But I really hate YAML. So another option, I think, could be a "component provider" system. User creates AppComponentProvider (or even adds an interface to their Kernel and uses that) with a registerComponents(ComponentRegistry $registry) where the can call things like $registry->add('Alert', VendorAlertComponent::class) or an ->addNamespace to register everything from a namespace with default names.

weaverryan avatar Feb 05 '24 21:02 weaverryan

I'd vote for doing things automatically, .... and add a non-configurable bundle prefix.

So vendor/acme/blog-bundle/Twig/Components/SuperWidget.php would be usable as

<twig:AcmeBlog:SuperWidget />

or (to be decided)

<twig:Acme:Blog:SuperWidget />

This is the more "obvious" / "standard" / "safe" way i think ....

If not, i like your provider idea, as long as they use a "prefix", "custom name", "namespace".

The alias idea is something we can (and will) do in a later move, but all this must be working without, so let's focus on that first. As the alias must only be a userland thing, not something a Bundle controls.

And we want at first give bundle a way to expose components.

smnandre avatar Feb 05 '24 21:02 smnandre

I'd vote for doing things automatically

This'd be my vote as well I think - would be very similar to twig templates in bundles. We would, I believe, still need a way for bundles to let the app know about their components. This could perhaps be the way they also let the app know about their anon components.

kbond avatar Feb 05 '24 21:02 kbond

In Twig, for templates it's done automatically for any Bundle indeed (registered in the prefixed namespace, and with the ! prefix to be able to reference the original bundle template even when it's overridden in app template/Bundle directories

All is done in two methods, we can do the exact same for both classes and templates

// src/Symfony/Bundle/TwigBundle/DependencyInjection/TwigExtension.php

// ...

 foreach ($this->getBundleTemplatePaths($container, $config) as $name => $paths) {
    $namespace = $this->normalizeBundleName($name);
    foreach ($paths as $path) {
        $twigFilesystemLoaderDefinition->addMethodCall('addPath', [$path, $namespace]);
    }

    if ($paths) {
        // the last path must be the bundle views directory
        $twigFilesystemLoaderDefinition->addMethodCall('addPath', [$path, '!'.$namespace]);
    }
}


// ... 


private function getBundleTemplatePaths(ContainerBuilder $container, array $config): array
{
    $bundleHierarchy = [];
    foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) {
        $defaultOverrideBundlePath = $container->getParameterBag()->resolveValue($config['default_path']).'/bundles/'.$name;

        if (file_exists($defaultOverrideBundlePath)) {
            $bundleHierarchy[$name][] = $defaultOverrideBundlePath;
        }
        $container->addResource(new FileExistenceResource($defaultOverrideBundlePath));

        if (file_exists($dir = $bundle['path'].'/Resources/views') || file_exists($dir = $bundle['path'].'/templates')) {
            $bundleHierarchy[$name][] = $dir;
        }
        $container->addResource(new FileExistenceResource($dir));
    }

    return $bundleHierarchy;
}

smnandre avatar Feb 05 '24 21:02 smnandre