[Live] Control the type of generated URLs #1398

1ed · 2024-01-16T19:35:30Z

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

Add a way to generate absolute URLs for live components.

My use case is that I embed a page with live components
into a site on a different host, so I need absolute URLs
for the components to work properly.

smnandre · 2024-01-16T22:44:03Z

Seems to me a very specific use case.. could it be handled with router configuration ?

src/LiveComponent/src/Attribute/AsLiveComponent.php

smnandre · 2024-01-16T22:45:36Z

src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php

            function (ChildDefinition $definition, AsLiveComponent $attribute) {
                $definition
-                    ->addTag('twig.component', array_filter($attribute->serviceConfig()))
+                    ->addTag('twig.component', array_filter($attribute->serviceConfig(), static fn ($v) => null !== $v))


Is this related to this PR ?

Without this, it does not work. It's a separate commit, so it can be cherry-picked as a bugfix or it can be moved to its own PR if needed.

Is there an error without it? I actually can't remember why we have that array_filter() at all

If I remove the array_filter it breaks the compiler pass in TwigComponent...

ux/src/TwigComponent/src/DependencyInjection/Compiler/TwigComponentPass.php

Line 48 in a2418ef

if (!\array_key_exists('key', $tag)) {

and other places where null is not checked.

Yes but would that break things now that you allow empty strings ?

I think it's not, all tests are green, but I can add '' !== $v too, if needed.

OK, I added back filtering out empty strings too.

Man it took me so much time to understand i'm sorry ..

I suggest that we handle that in another PR, as it's clearly not in LiveComponent that we should handle this.

#1402

1ed

Seems to me a very specific use case.. could it be handled with router configuration ?

IDK maybe.. what do you have in mind?

If you suggesting to use the host parameter it doesn't work, because from Symfony's point of view the host is the same as the request comes in to.

1ed · 2024-01-17T08:20:47Z

src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php

            function (ChildDefinition $definition, AsLiveComponent $attribute) {
                $definition
-                    ->addTag('twig.component', array_filter($attribute->serviceConfig()))
+                    ->addTag('twig.component', array_filter($attribute->serviceConfig(), static fn ($v) => null !== $v))


Without this, it does not work. It's a separate commit, so it can be cherry-picked as a bugfix or it can be moved to its own PR if needed.

src/LiveComponent/src/Attribute/AsLiveComponent.php

weaverryan

This looks sensible to me!

src/LiveComponent/src/Attribute/AsLiveComponent.php

weaverryan · 2024-01-17T17:44:12Z

src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php

            function (ChildDefinition $definition, AsLiveComponent $attribute) {
                $definition
-                    ->addTag('twig.component', array_filter($attribute->serviceConfig()))
+                    ->addTag('twig.component', array_filter($attribute->serviceConfig(), static fn ($v) => null !== $v))


Is there an error without it? I actually can't remember why we have that array_filter() at all

smnandre · 2024-01-17T19:37:53Z

Wait, so you'll POST from domainA.com to domainB.com ?

Should this be on a component - per - component basis... or a config wide one (that seem more logical to me) ?

1ed · 2024-01-17T19:44:45Z

Wait, so you'll POST from domainA.com to domainB.com ?

Should this be on a component - per - component basis... or a config wide one (that seem more logical to me) ?

As the route can be configured on the component level, for me it is logical to do the same with the URL generated for that route.

smnandre · 2024-01-17T19:58:03Z

As the route can be configured on the component level, for me it is logical to do the same with the URL generated for that route.

Yes i mean, should this be possible to config it on an app-level too, as the route can be configured there for all components ?

Last thing, is there any implication/effect with the "liveQuery" thing ?

1ed · 2024-01-17T20:42:13Z

As the route can be configured on the component level, for me it is logical to do the same with the URL generated for that route.

Yes i mean, should this be possible to config it on an app-level too, as the route can be configured there for all components ?

Ah, I see. That would mean we need to introduce configuration for the live component and add a config entry for it, e.g. something like

live_component:
    default_url_reference_type: 'absolute_path' # support strings and UrlGeneratorInterface consts too -> map strings back to consts

read the configuration and use this as the default value for the metadata (the default in AsLiveComponent should be null) e.g. in LiveComponentMetadataFactory. That can be done, maybe in a different PR.

Last thing, is there any implication/effect with the "liveQuery" thing ?

I'm not sure, I'm not familiar with that, but I took a look at the PR and I think using the browser's history api and getting query string values from the request should not be affected by this.

smnandre · 2024-01-18T01:07:21Z

Could you add a test with an action (just to be sure everything work with in JS and in the request handling ?

(and that way we'd have a regression test later on this)

1ed · 2024-01-19T09:44:37Z

Could you add a test with an action (just to be sure everything work with in JS and in the request handling ?

(and that way we'd have a regression test later on this)

I just added a test for that (but without js, for that we would need to setup panther, I think).

1ed · 2024-01-22T14:34:10Z

I think this is ready. Can we merge it in this form or it needs more work?

weaverryan · 2024-01-22T16:29:20Z

Thanks Gábor!

1ed force-pushed the live-abs-url branch from 3c0d084 to adb0d12 Compare January 16, 2024 19:42

smnandre reviewed Jan 16, 2024

View reviewed changes

src/LiveComponent/src/Attribute/AsLiveComponent.php Show resolved Hide resolved

smnandre reviewed Jan 16, 2024

View reviewed changes

1ed commented Jan 17, 2024

View reviewed changes

weaverryan reviewed Jan 17, 2024

View reviewed changes

1ed force-pushed the live-abs-url branch from adb0d12 to 2d371af Compare January 17, 2024 19:54

1ed force-pushed the live-abs-url branch 2 times, most recently from c60e5bf to 69ef030 Compare January 22, 2024 09:29

weaverryan approved these changes Jan 22, 2024

View reviewed changes

[Live] Control the type of generated URLs

7e91f1f

weaverryan force-pushed the live-abs-url branch from 69ef030 to 7e91f1f Compare January 22, 2024 16:29

weaverryan merged commit b73eac8 into symfony:2.x Jan 22, 2024

Uh oh!

[Live] Control the type of generated URLs #1398

[Live] Control the type of generated URLs #1398

Uh oh!

Conversation

1ed commented Jan 16, 2024

Uh oh!

smnandre commented Jan 16, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1ed Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1ed left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

1ed Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weaverryan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smnandre commented Jan 17, 2024

Uh oh!

1ed commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smnandre commented Jan 17, 2024

Uh oh!

1ed commented Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smnandre commented Jan 18, 2024

Uh oh!

1ed commented Jan 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

1ed commented Jan 22, 2024

Uh oh!

weaverryan commented Jan 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

1ed Jan 17, 2024 •

edited

Loading

1ed left a comment •

edited

Loading

1ed Jan 17, 2024 •

edited

Loading

1ed commented Jan 17, 2024 •

edited

Loading

1ed commented Jan 17, 2024 •

edited

Loading

1ed commented Jan 19, 2024 •

edited

Loading