Skip to content

Conversation

@weaverryan
Copy link
Member

@weaverryan weaverryan commented Mar 16, 2023

Hi!

Subtle change. Previously, the webpack loader parsed controllers.json (which eventually becomes the symfonyControllers variable in index.ts) into something that exported an object where each key was a promise - something like:

export default {
    'symfony--ux-autocomplete--autocomplete': import(/* webpackMode: \"eager\" */ '@symfony/ux-autocomplete/dist/controller.js')
}

Then, in startStimulusApp(), we called used .then() to wait for that promise to resolve then registered its resolved value:

symfonyControllers[controllerName].then((module) => {
    application.register(controllerName, module.default);
});

This is totally unnecessary and a relic of how this library was originally built. It also causes the UX controllers to be registered late. It's barely noticeable, but in practice, instead of the UX controllers being registered BEFORE the DOM is ready, they are registered after. This actually causes a problem with a new feature from LiveComponents.

The new parsed version of controllers.json from the loader looks much simpler:

+ import controller_0 from '@symfony/ux-autocomplete/dist/controller.js';

export default {
-    'symfony--ux-autocomplete--autocomplete': import(/* webpackMode: \"eager\" */ '@symfony/ux-autocomplete/dist/controller.js')
+    'symfony--ux-autocomplete--autocomplete': controller_0,
}

(I don't show it here, but lazy controllers have a similar simplification).

In short: it's a bug fix & an easy win. Controllers will load slightly earlier as a result.

Thanks!

@weaverryan weaverryan force-pushed the removing-unnecessary-promise branch from f7a10fd to 0d77e41 Compare March 20, 2023 19:22
@weaverryan weaverryan merged commit b49d87b into symfony:main Mar 20, 2023
weaverryan added a commit to symfony/ux that referenced this pull request Mar 22, 2023
This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Site] Upgrading to latest version + other changes

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Tickets       | None
| License       | MIT

* Updating to latest version of UX (which will become 2.8.0)
* Upgrading deps
* Various small changes and fixes

TODO:
* [x] Use symfony/stimulus-bridge#81 when it's available

Commits
-------

6c9675b [Site] Upgrading to latest version + other changes
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 this pull request may close these issues.

2 participants