-
-
Notifications
You must be signed in to change notification settings - Fork 398
[Translator] Add option ux_translator.dump_typescript to enable/disable TypeScript types generation
#3218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
7d77142 to
021db9e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new configuration option dump_typescript to the UX Translator component, allowing developers to disable TypeScript type definitions generation. This is particularly useful in production environments when using AssetMapper, where TypeScript types are not needed, resulting in significantly faster cache clears (from ~1min to ~10s for applications with ~3320 translation messages).
Key changes:
- Added
dump_typescriptboolean configuration option (defaults totrue) - Modified
TranslationsDumperto conditionally generate TypeScript.d.tsfiles based on the configuration - Updated tests to verify the new behavior and refactored to remove shared test setup
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Translator/src/TranslationsDumper.php | Added dumpTypeScript parameter and conditional logic to skip TypeScript file generation when disabled |
| src/Translator/src/DependencyInjection/Configuration.php | Added dump_typescript boolean configuration node with default value true |
| src/Translator/src/DependencyInjection/UxTranslatorExtension.php | Wired the new configuration value to the dumper service |
| src/Translator/config/services.php | Updated service definition to include the new dump_typescript abstract argument |
| src/Translator/tests/TranslationsDumperTest.php | Added test for disabled TypeScript generation and refactored tests to instantiate dumper per test |
| src/Translator/doc/index.rst | Added documentation section explaining how to disable TypeScript types in production |
| src/Translator/CHANGELOG.md | Documented the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
021db9e to
a2cc3dc
Compare
…sages, if no `{` is found (Kocal)
This PR was merged into the 2.x branch.
Discussion
----------
[Translator] Early exit parameters extraction from Intl messages, if no `{` is found
| Q | A
| -------------- | ---
| Bug fix? | yes
| New feature? | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md -->
| Documentation? | no <!-- required for new features, or documentation updates -->
| Issues | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License | MIT
Following #3218
On an app with ~3320 keys, ~2870 of them didn't use the ICU syntax. Early exiting when no `{` reduced the cache-clear time by ~60%:
[](https://blackfire.io/profiles/compare/209538ae-26bf-40bf-a3f4-5a51d91a4fe7/graph)
Commits
-------
c8a303c [Translator] Early exit parameters extraction from Intl messages, if no `{` is found
…able TypeScript types generation
a2cc3dc to
741b421
Compare
| ->defaultValue('%kernel.project_dir%/var/translations') | ||
| ->end() | ||
| ->booleanNode('dump_typescript') | ||
| ->info('Control if TypeScript types should be dumped alongside translations. Can be useful to disable when not using TypeScript (e.g. AssetMapper in production).') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ->info('Control if TypeScript types should be dumped alongside translations. Can be useful to disable when not using TypeScript (e.g. AssetMapper in production).') | |
| ->info('Control whether TypeScript types are dumped alongside translations. Disable this if you do not use TypeScript (e.g. in production when using AssetMapper).') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #3221
| private string $dumpDir, | ||
| private bool $dumpTypeScript, | ||
| private MessageParametersExtractor $messageParametersExtractor, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I now this class is marked @experimental but the component is out there for some times now ... :/
…ription (Kocal) This PR was merged into the 2.x branch. Discussion ---------- [Translator] Reword `dump_typescript` option description | Q | A | -------------- | --- | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | Deprecations? | no <!-- if yes, also update UPGRADE-*.md and src/**/CHANGELOG.md --> | Documentation? | yes <!-- required for new features, or documentation updates --> | Issues | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT Following #3218 (review), thank you `@smnandre` The recipe symfony/recipes#1499 have been updated as well. Commits ------- 248f7fc [Translator] Reword `dump_typescript` option description
Recipe PR: symfony/recipes#1499
When using the UX Translator with the AssetMapper, there is no point to dump TypeScript types in production, since these files will not be used. The deployment will be faster.
The new option
dump_typescriptallows to disable this behavior, default totrue.On an app with ~3320 translation messages, a cache clear goes from ~1min to ~10s, see https://blackfire.io/profiles/compare/4d9553ca-7b96-415d-aa0f-2991f721b609/graph:

~50 sec for extracting types, especially when using ICU translations, is too much, even for ~3k keys. I think we can have a quick-win by early exiting when no
{is found in the translation message.