[Translator] Fix changing dump directory using AssetMapper #1904
Conversation
carsonbot
added
Status: Reviewed
| $container->prependExtensionConfig('framework', [ | ||
| 'asset_mapper' => [ | ||
| 'paths' => [ | ||
| __DIR__.'/../../assets/dist' => '@symfony/ux-translator', | ||
| '%kernel.project_dir%/var/translations' => 'var/translations', | ||
| $config['dump_directory'] => '@app/translations', |
There was a problem hiding this comment.
Could you add a functionnal test for this ? That will prevent any future regression
There was a problem hiding this comment.
Not sure how to test for this. I don't think I can access the config, and I can't functionally test this without requiring asset mapper in require-dev, which is not wanted. If you have any input on how I can retrieve the config from the test scope I'd be glad to hear about it :)
There was a problem hiding this comment.
I shouldn't have ignored this conversation 😭
|
Thank you @maelanleborgne. |
|
Well.... bad week for UX 🐞 🐞 🐞 Here is my importmap 
I did a composer update and...
To fix i just needed to replace my "var/" by "@app" links ... (and then it seems cool thanks @maelanleborgne) But we cannot do this like that//// or this will crash for everyone (with no clue/help how to fix) So can we find something today.. or should we revert before the complaints start ? 😅 |
|
Seems like this is caused by a small misleading info from the doc on what lines to add in '@app/translations' => [
'path' => 'var/translations/index.js',
],
'@app/translations/configuration' => [
'path' => 'var/translations/configuration.js',
],but importmap usually uses relative paths '@app/translations' => [
'path' => './var/translations/index.js',
],
'@app/translations/configuration' => [
'path' => './var/translations/configuration.js',
],This worked because the associated 'namespace' in $container->prependExtensionConfig('framework', [
'asset_mapper' => [
'paths' => [
__DIR__.'/../../assets/dist' => '@symfony/ux-translator',
$config['dump_directory'] => '@app/translations',
$config['dump_directory'] => 'var/translations', // or '%kernel.project_dir%/var/translations' => 'var/translations'
],
],Anyway, we can revert for now, I'll go fix the doc in the meantime. |
…Mapper (maelanleborgne) This PR was merged into the 2.x branch. Discussion ---------- [Translator] Revert Fix changing dump directory using AssetMapper | Q | A | ------------- | --- | Bug fix? | yes | New feature? |no | Issues | #1904 (comment) | License | MIT [This PR ](#1904) introduced a bug : when using 'var/translations' in the `importmap.php` file instead of a relative path, it would break the twig `importmap()` function. ~~This PR aims to rectify this regression by keeping a 'var/translations' namespace pointing to the dump_directory. If this is not accepted, the previous PR should be reverted to prevent users using 'var/translations' in there `importmap.php` to face this issue, but the documentation should still be updated to display an example using the relative path instead of a misleading namespace.~~ This PR reverts the change. A word in the doc to say that the `dump_directory` conf is not compatible with asset_mapper would be nice. Commits ------- e551cff Revert "Fix changing dump directory using AssetMapper"
The configuration node
ux_translator.dump_directorywasn't correctly taken into account when using UxTranslator with AssetMapper.This PR fixes the configuration prepend of AssetMapper to use the custom path + adds a "caution" section in the docs to remind developers to also update their
importmap.php.