search

symfony / ux

Symfony UX initiative: a JavaScript ecosystem for Symfony
https://ux.symfony.com/
MIT License
858 stars 315 forks source link

[StimulusBundle] Check controllers source files for laziness #2304

Closed MatTheCat closed 3 weeks ago

MatTheCat commented 1 month ago
Q A
Bug fix? no
New feature? yes
Issues Fix https://github.com/sensiolabs/minify-bundle/issues/10
License MIT

The StimulusBundle allows to mark a controller as lazy using a /* stimulusFetch: 'lazy' */ comment, but it will be searched in said controller’s compiled content. If the compilation removes that comment (it typically happens when using minifiers), then the controller is no longer considered lazy by the ControllersMapGenerator.

This PR makes the comment searched in source files, so that its presence doesn’t depend on the compilation’s result.

MatTheCat commented 1 month ago

I guess it would break compilers wanting to interact with the StimulusBundle by adding or removing the /* stimulusFetch: 'lazy' */ comment in controllers. Wouldn't it be better to decorate or replace the stimulus.asset_mapper.controllers_map_generator service in that case?

smnandre commented 4 weeks ago

It feels to me a very internal matter... and a very logical change.

To make one of your custom controllers lazy, add a special comment on top: [...]

To me it's never an AssetMapper thing, but a source code thing, so..

I'd even call this a feature to be honest , as it would remove the following constraint:

[!NOTE] If you write your controllers using TypeScript, make sure removeComments is not set to true in your TypeScript config.

https://symfony.com/bundles/StimulusBundle/current/index.html#lazy-stimulus-controllers

So i'm clearly 👍 on this

@MatTheCat Do you think you could add a test or two ?

MatTheCat commented 4 weeks ago

Test added!

Kocal commented 3 weeks ago

Thanks!

javiereguiluz commented 3 weeks ago

Thanks for fixing this bug Mathieu.