[AssetMapper] Make asset-map:compile clean assets in debug mode, require --force to generate them
#59073
Conversation
|
|
||
| foreach ($this->assetMapper->allAssets() as $asset) { | ||
| if (is_file($publicDir.'/'.$asset->publicPath)) { | ||
| unlink($publicDir.'/'.$asset->publicPath); |
There was a problem hiding this comment.
Maybe, the FileSystem component could be used here ? (With exists and remove functions) ?
There was a problem hiding this comment.
Not without reasons. What would be yours?
There was a problem hiding this comment.
Hello, it handles issues in case of errors like "impossible" to remove a directory and raise an IOException well formatted and exists handle cases like PHP_MAXPATHLEN, I think the lib is made to ease developer life, isn't it ? :)
asset-map:compile clean assets in debug mode, require --force to generate them
src/Symfony/Component/AssetMapper/Command/AssetMapperCompileCommand.php
Outdated
Show resolved
Hide resolved
| * Make `asset-map:compile` remove assets in debug mode | ||
| * Add `--force` to the `asset-map:compile` command to make it generate assets in debug mode |
There was a problem hiding this comment.
It would not have been easier to get / use with an option "--purge" + keeping the current behaviour ?
A compile command that clear files feels very weird .. using different arguments to do the same thing also 🤷
There was a problem hiding this comment.
My reasoning is that in dev mode you don't want the assets to block dynamic generation, yet this is the default right now, and this serves no DX purpose.
There was a problem hiding this comment.
Current DX can be improved for users after they compile assets locally, yes.
But this PR can also make things worse for all the others.. Two opposite behaviour between debug and not debug.. i'm not sure this will be a DX win. 🤷♂️
Two commands: asset-map:compile and asset-map:purge (or asset-map:clear) feels easier to explain, document, use.
Just a personal feeling.
There was a problem hiding this comment.
Please explain what you mean by "worse".
The behavior I'm proposing here is not opposite, it's consistent with what each mode (debug/non-debug) needs to work. Adding a myriad of commands for fine grained operations will just make things more complex, aka worse, when we know what the behavior should be to make things Just Work (TM)
|
My answer here to explain my opinion on these PRs: compiled is not only cached |
…quire --force to generate them
ca4b653 to
304cf4f
Compare
|
Implémentation comment: doing assetMapper->allAsset() will rebuild all the assets (as the config has been removed) to then delete them. And wont delete any asset that changed. Also without the event beeing triggered the result will change (at least a bit). You way need to delete all the folder (?) or use the manifest generated (easier and quicker but other files can be in here too) |
|
Let me come back later on this topic. |
Related to #59072, the goal is to improve the DX and allow adding the command to the default recipe.
This doesn't fully fix the issue #59072 - a cache warmer would be required for a full DX improvement.