[LiveComponent] Replace the browser's URL before triggering render:finished hook

[xDeSwa]

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

After the URL change, performing the render:finished event will provide smoother control.

[github-actions]

📊 Packages dist files size difference

ℹ️ No difference in dist packagesFiles.

[Kocal]

Makes sense to me

[smnandre]

Can we movev the liveUrl update in processRerender instead ?

[Kocal]

We should be careful on how we do it, because the method contains some early returns that could prevent the URL update

[xDeSwa]

done. moved into processRerender

[Kocal]

But now people hooking on render:started and passing shouldRender = false won't see the URL updated anymore, that's a BC

[smnandre]

Yep indeed.. but if I get this right, with this PR the hook would be trigger even when the render is skipped or if an error occurs .. and this would be some BC too :| Or do we consider this a bug ?

But there are at least two places we need to check attentively how it would impact codebases:

This one (seems without effect but i'd like to be sure):

        component.on('render:finished', () => {
            // re-start polling, in case polling changed
            this.initializePolling();
        });

/src/LiveComponent/assets/src/Component/plugins/PollingPlugin.ts#L21-L24

But this one makes me more hesitant:

    attachToComponent(component: Component): void {
        this.synchronizeValueOfModelFields(component);
        component.on('render:finished', () => {
            this.synchronizeValueOfModelFields(component);
        });

src/LiveComponent/assets/src/Component/plugins/SetValueOntoModelFieldsPlugin.ts#L16-L18

[xDeSwa]

@Kocal | @smnandre
URL change method moved before ReRender function.

Since no backendResponse data changes are made within the ReRender function, changing the URL before the function makes sense. This will avoid any hitches to hook operations and ensure access to the correct URL within the hook.

[smnandre]

Seems ok to me! Can you fix fabbot check ? (remove the merge commit and rebase on 2.x) .. or we won't be able to merge 😅

[xDeSwa]

@smnandre is it ok now? thanks

[smnandre]

Well, i do not see any red in the checks box so .. yeah 😄

[smnandre]

Thank you @xDeSwa !!

[Kocal]

Thank you @xDeSwa.