Skip to content

Commit c8a47ec

Browse files
KocalKocal
committed
feature #3244 [Translator] Refactor TranslationsDumper options from __constructor and setters, to dump method (Kocal)
This PR was merged into the 2.x branch. Discussion ---------- [Translator] Refactor `TranslationsDumper` options from `__constructor` and setters, to `dump` method | Q | A | -------------- | --- | Bug fix? | no | New feature? | yes <!-- 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 This PR simplifies how `TranslationsDumper` works by moving all configuration options from the constructor/setters into the `dump()` method itself. Even if using the `TranslationsDumper` directly is not a common use case (most users will the default behavior with the cache warmer), this change improves the overall design of the class: - no stateful instance - only one place to configure settings, instead of __construct() and setters - adding new options will be easier in the future Promise, this is the last BC for Translator 🙏🏻 Commits ------- cbed37a [Translator] Refactor `TranslationsDumper` options from `__constructor` and setters, to `dump` method
2 parents 356ee98 + cbed37a commit c8a47ec

File tree

7 files changed

+114
-68
lines changed

7 files changed

+114
-68
lines changed

‎src/Translator/CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@
4949

5050
**Note:** This is a breaking change, but the UX Translator component is still experimental.
5151

52+
- **[BC BREAK]** Refactor `TranslationsDumper` to accept configuration options via `dump()` method parameters, instead of constructor arguments or method calls:
53+
- Removed `$dumpDir` and `$dumpTypeScript` constructor arguments
54+
- Removed `TranslationsDumper::addIncludedDomain()` and `TranslationsDumper::addExcludedDomain()` methods
55+
56+
**Note:** This is a breaking change, but the UX Translator component is still experimental.
57+
5258
- Add configuration `ux_translator.dump_typescript` to enable/disable TypeScript types dumping,
5359
default to `true`. Generating TypeScript types is useful when developing,
5460
but not in production when using the AssetMapper (which does not use these types).

‎src/Translator/config/services.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,15 @@
2626
->args([
2727
service('translator'),
2828
service('ux.translator.translations_dumper'),
29+
abstract_arg('dump_directory'),
30+
abstract_arg('dump_typescript'),
31+
abstract_arg('included_domains'),
32+
abstract_arg('excluded_domains'),
2933
])
3034
->tag('kernel.cache_warmer')
3135

3236
->set('ux.translator.translations_dumper', TranslationsDumper::class)
3337
->args([
34-
abstract_arg('dump_directory'),
35-
abstract_arg('dump_typescript'),
3638
service('ux.translator.message_parameters.extractor.message_parameters_extractor'),
3739
service('ux.translator.message_parameters.extractor.intl_message_parameters_extractor'),
3840
service('ux.translator.message_parameters.printer.typescript_message_parameters_printer'),

‎src/Translator/src/CacheWarmer/TranslationsCacheWarmer.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,23 @@
1818
/**
1919
* @author Hugo Alliaume <[email protected]>
2020
*
21+
* @internal
22+
*
2123
* @experimental
2224
*/
2325
class TranslationsCacheWarmer implements CacheWarmerInterface
2426
{
27+
/**
28+
* @param list<string> $includedDomains
29+
* @param list<string> $excludedDomains
30+
*/
2531
public function __construct(
2632
private TranslatorBagInterface $translatorBag,
2733
private TranslationsDumper $translationsDumper,
34+
private string $dumpDir,
35+
private bool $dumpTypeScript,
36+
private array $includedDomains,
37+
private array $excludedDomains,
2838
) {
2939
}
3040

@@ -36,7 +46,11 @@ public function isOptional(): bool
3646
public function warmUp(string $cacheDir, ?string $buildDir = null): array
3747
{
3848
$this->translationsDumper->dump(
39-
...$this->translatorBag->getCatalogues()
49+
$this->translatorBag->getCatalogues(),
50+
$this->dumpDir,
51+
$this->dumpTypeScript,
52+
$this->includedDomains,
53+
$this->excludedDomains,
4054
);
4155

4256
// No need to preload anything

‎src/Translator/src/DependencyInjection/UxTranslatorExtension.php‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,22 @@ public function load(array $configs, ContainerBuilder $container): void
3535
$loader = (new PhpFileLoader($container, new FileLocator(\dirname(__DIR__).'/../config')));
3636
$loader->load('services.php');
3737

38-
$dumperDefinition = $container->getDefinition('ux.translator.translations_dumper');
39-
$dumperDefinition->setArgument(0, $config['dump_directory']);
40-
$dumperDefinition->setArgument(1, $config['dump_typescript']);
38+
$includedDomains = [];
39+
$excludedDomains = [];
4140

4241
if (isset($config['domains'])) {
43-
$method = 'inclusive' === $config['domains']['type'] ? 'addIncludedDomain' : 'addExcludedDomain';
44-
foreach ($config['domains']['elements'] as $domainName) {
45-
$dumperDefinition->addMethodCall($method, [$domainName]);
42+
if ('inclusive' === $config['domains']['type']) {
43+
$includedDomains = $config['domains']['elements'];
44+
} else {
45+
$excludedDomains = $config['domains']['elements'];
4646
}
4747
}
48+
49+
$cacheWarmerDefinition = $container->getDefinition('ux.translator.cache_warmer.translations_cache_warmer');
50+
$cacheWarmerDefinition->setArgument(2, $config['dump_directory']);
51+
$cacheWarmerDefinition->setArgument(3, $config['dump_typescript']);
52+
$cacheWarmerDefinition->setArgument(4, $includedDomains);
53+
$cacheWarmerDefinition->setArgument(5, $excludedDomains);
4854
}
4955

5056
public function prepend(ContainerBuilder $container): void

‎src/Translator/src/TranslationsDumper.php‎

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -30,24 +30,33 @@
3030
*/
3131
class TranslationsDumper
3232
{
33-
private array $excludedDomains = [];
34-
private array $includedDomains = [];
35-
3633
public function __construct(
37-
private string $dumpDir,
38-
private bool $dumpTypeScript,
3934
private MessageParametersExtractor $messageParametersExtractor,
4035
private IntlMessageParametersExtractor $intlMessageParametersExtractor,
4136
private TypeScriptMessageParametersPrinter $typeScriptMessageParametersPrinter,
4237
private Filesystem $filesystem,
4338
) {
4439
}
4540

46-
public function dump(MessageCatalogueInterface ...$catalogues): void
47-
{
48-
$this->filesystem->mkdir($this->dumpDir);
49-
$this->filesystem->remove($fileIndexJs = $this->dumpDir.'/index.js');
50-
$this->filesystem->remove($fileIndexDts = $this->dumpDir.'/index.d.ts');
41+
/**
42+
* @param list<MessageCatalogueInterface> $catalogues
43+
* @param list<Domain> $includedDomains
44+
* @param list<Domain> $excludedDomains
45+
*/
46+
public function dump(
47+
array $catalogues,
48+
string $dumpDir,
49+
bool $dumpTypeScript = true,
50+
array $includedDomains = [],
51+
array $excludedDomains = [],
52+
): void {
53+
if ($includedDomains && $excludedDomains) {
54+
throw new \LogicException('You cannot set both "excluded_domains" and "included_domains" at the same time.');
55+
}
56+
57+
$this->filesystem->mkdir($dumpDir);
58+
$this->filesystem->remove($fileIndexJs = $dumpDir.'/index.js');
59+
$this->filesystem->remove($fileIndexDts = $dumpDir.'/index.d.ts');
5160

5261
$this->filesystem->appendToFile(
5362
$fileIndexJs,
@@ -58,10 +67,10 @@ public function dump(MessageCatalogueInterface ...$catalogues): void
5867
export const messages = {
5968

6069
JS,
61-
json_encode($this->getLocaleFallbacks(...$catalogues), \JSON_THROW_ON_ERROR)
70+
json_encode($this->getLocaleFallbacks($catalogues), \JSON_THROW_ON_ERROR)
6271
));
6372

64-
if ($this->dumpTypeScript) {
73+
if ($dumpTypeScript) {
6574
$this->filesystem->appendToFile(
6675
$fileIndexDts,
6776
<<<'TS'
@@ -75,7 +84,7 @@ public function dump(MessageCatalogueInterface ...$catalogues): void
7584
);
7685
}
7786

78-
foreach ($this->getTranslations(...$catalogues) as $translationId => $translationsByDomainAndLocale) {
87+
foreach ($this->getTranslations($catalogues, $excludedDomains, $includedDomains) as $translationId => $translationsByDomainAndLocale) {
7988
$translationId = str_replace('"', '\\"', $translationId);
8089
$this->filesystem->appendToFile($fileIndexJs, \sprintf(
8190
' "%s": %s,%s',
@@ -84,7 +93,7 @@ public function dump(MessageCatalogueInterface ...$catalogues): void
8493
"\n"
8594
));
8695

87-
if ($this->dumpTypeScript) {
96+
if ($dumpTypeScript) {
8897
$this->filesystem->appendToFile($fileIndexDts, \sprintf(
8998
' "%s": %s;%s',
9099
$translationId,
@@ -96,41 +105,29 @@ public function dump(MessageCatalogueInterface ...$catalogues): void
96105

97106
$this->filesystem->appendToFile($fileIndexJs, '};'."\n");
98107

99-
if ($this->dumpTypeScript) {
108+
if ($dumpTypeScript) {
100109
$this->filesystem->appendToFile($fileIndexDts, '};'."\n");
101110
}
102111
}
103112

104-
public function addExcludedDomain(string $domain): void
105-
{
106-
if ($this->includedDomains) {
107-
throw new \LogicException('You cannot set both "excluded_domains" and "included_domains" at the same time.');
108-
}
109-
$this->excludedDomains[] = $domain;
110-
}
111-
112-
public function addIncludedDomain(string $domain): void
113-
{
114-
if ($this->excludedDomains) {
115-
throw new \LogicException('You cannot set both "excluded_domains" and "included_domains" at the same time.');
116-
}
117-
$this->includedDomains[] = $domain;
118-
}
119-
120113
/**
114+
* @param list<MessageCatalogueInterface> $catalogues
115+
* @param list<Domain> $excludedDomains
116+
* @param list<Domain> $includedDomains
117+
*
121118
* @return array<MessageId, array<Domain, array<Locale, string>>>
122119
*/
123-
private function getTranslations(MessageCatalogueInterface ...$catalogues): array
120+
private function getTranslations(array $catalogues, array $excludedDomains, array $includedDomains): array
124121
{
125122
$translations = [];
126123

127124
foreach ($catalogues as $catalogue) {
128125
$locale = $catalogue->getLocale();
129126
foreach ($catalogue->getDomains() as $domain) {
130-
if (\in_array($domain, $this->excludedDomains, true)) {
127+
if (\in_array($domain, $excludedDomains, true)) {
131128
continue;
132129
}
133-
if ($this->includedDomains && !\in_array($domain, $this->includedDomains, true)) {
130+
if ($includedDomains && !\in_array($domain, $includedDomains, true)) {
134131
continue;
135132
}
136133
foreach ($catalogue->all($domain) as $id => $message) {
@@ -185,7 +182,10 @@ private function getTranslationsTypeScriptTypeDefinition(array $translationsByDo
185182
);
186183
}
187184

188-
private function getLocaleFallbacks(MessageCatalogueInterface ...$catalogues): array
185+
/**
186+
* @param list<MessageCatalogueInterface> $catalogues
187+
*/
188+
private function getLocaleFallbacks(array $catalogues): array
189189
{
190190
$localesFallbacks = [];
191191

‎src/Translator/tests/CacheWarmer/TranslationsCacheWarmerTest.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,15 +42,24 @@ public function test()
4242
])
4343
);
4444

45+
$dumpDir = '/tmp/translations';
46+
$dumpTypeScript = true;
47+
$includedDomains = [];
48+
$excludedDomains = [];
49+
4550
$translationsDumperMock = $this->createMock(TranslationsDumper::class);
4651
$translationsDumperMock
4752
->expects($this->once())
4853
->method('dump')
49-
->with(...$translatorBag->getCatalogues());
54+
->with($translatorBag->getCatalogues(), $dumpDir, $dumpTypeScript, $includedDomains, $excludedDomains);
5055

5156
$translationsCacheWarmer = new TranslationsCacheWarmer(
5257
$translatorBag,
53-
$translationsDumperMock
58+
$translationsDumperMock,
59+
$dumpDir,
60+
$dumpTypeScript,
61+
$includedDomains,
62+
$excludedDomains
5463
);
5564

5665
$translationsCacheWarmer->warmUp(self::$cacheDir);

‎src/Translator/tests/TranslationsDumperTest.php‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,14 +36,15 @@ public static function tearDownAfterClass(): void
3636
public function testDump()
3737
{
3838
$translationsDumper = new TranslationsDumper(
39-
self::$translationsDumpDir,
40-
true,
4139
new MessageParametersExtractor(),
4240
new IntlMessageParametersExtractor(),
4341
new TypeScriptMessageParametersPrinter(),
4442
new Filesystem(),
4543
);
46-
$translationsDumper->dump(...self::getMessageCatalogues());
44+
$translationsDumper->dump(
45+
catalogues: self::getMessageCatalogues(),
46+
dumpDir: self::$translationsDumpDir,
47+
);
4748

4849
$this->assertFileExists(self::$translationsDumpDir.'/index.js');
4950
$this->assertFileExists(self::$translationsDumpDir.'/index.d.ts');
@@ -113,14 +114,16 @@ public function testDump()
113114
public function testShouldNotDumpTypeScriptTypes()
114115
{
115116
$translationsDumper = new TranslationsDumper(
116-
self::$translationsDumpDir,
117-
false,
118117
new MessageParametersExtractor(),
119118
new IntlMessageParametersExtractor(),
120119
new TypeScriptMessageParametersPrinter(),
121120
new Filesystem(),
122121
);
123-
$translationsDumper->dump(...self::getMessageCatalogues());
122+
$translationsDumper->dump(
123+
catalogues: self::getMessageCatalogues(),
124+
dumpDir: self::$translationsDumpDir,
125+
dumpTypeScript: false,
126+
);
124127

125128
$this->assertFileExists(self::$translationsDumpDir.'/index.js');
126129
$this->assertFileDoesNotExist(self::$translationsDumpDir.'/index.d.ts');
@@ -129,16 +132,17 @@ public function testShouldNotDumpTypeScriptTypes()
129132
public function testDumpWithExcludedDomains()
130133
{
131134
$translationsDumper = new TranslationsDumper(
132-
self::$translationsDumpDir,
133-
true,
134135
new MessageParametersExtractor(),
135136
new IntlMessageParametersExtractor(),
136137
new TypeScriptMessageParametersPrinter(),
137138
new Filesystem(),
138139
);
139-
$translationsDumper->addExcludedDomain('foobar');
140140

141-
$translationsDumper->dump(...self::getMessageCatalogues());
141+
$translationsDumper->dump(
142+
catalogues: self::getMessageCatalogues(),
143+
dumpDir: self::$translationsDumpDir,
144+
excludedDomains: ['foobar'],
145+
);
142146

143147
$this->assertFileExists(self::$translationsDumpDir.'/index.js');
144148
$this->assertStringNotContainsString('foobar', file_get_contents(self::$translationsDumpDir.'/index.js'));
@@ -147,16 +151,17 @@ public function testDumpWithExcludedDomains()
147151
public function testDumpIncludedDomains()
148152
{
149153
$translationsDumper = new TranslationsDumper(
150-
self::$translationsDumpDir,
151-
true,
152154
new MessageParametersExtractor(),
153155
new IntlMessageParametersExtractor(),
154156
new TypeScriptMessageParametersPrinter(),
155157
new Filesystem(),
156158
);
157-
$translationsDumper->addIncludedDomain('messages');
158159

159-
$translationsDumper->dump(...self::getMessageCatalogues());
160+
$translationsDumper->dump(
161+
catalogues: self::getMessageCatalogues(),
162+
dumpDir: self::$translationsDumpDir,
163+
includedDomains: ['messages'],
164+
);
160165

161166
$this->assertFileExists(self::$translationsDumpDir.'/index.js');
162167
$this->assertStringNotContainsString('foobar', file_get_contents(self::$translationsDumpDir.'/index.js'));
@@ -168,16 +173,18 @@ public function testSetBothIncludedAndExcludedDomains()
168173
$this->expectExceptionMessage('You cannot set both "excluded_domains" and "included_domains" at the same time.');
169174

170175
$translationsDumper = new TranslationsDumper(
171-
self::$translationsDumpDir,
172-
true,
173176
new MessageParametersExtractor(),
174177
new IntlMessageParametersExtractor(),
175178
new TypeScriptMessageParametersPrinter(),
176179
new Filesystem(),
177180
);
178181

179-
$translationsDumper->addIncludedDomain('foobar');
180-
$translationsDumper->addExcludedDomain('messages');
182+
$translationsDumper->dump(
183+
catalogues: self::getMessageCatalogues(),
184+
dumpDir: self::$translationsDumpDir,
185+
includedDomains: ['foobar'],
186+
excludedDomains: ['messages'],
187+
);
181188
}
182189

183190
public function testSetBothExcludedAndIncludedDomains()
@@ -186,15 +193,17 @@ public function testSetBothExcludedAndIncludedDomains()
186193
$this->expectExceptionMessage('You cannot set both "excluded_domains" and "included_domains" at the same time.');
187194

188195
$translationsDumper = new TranslationsDumper(
189-
self::$translationsDumpDir,
190-
true,
191196
new MessageParametersExtractor(),
192197
new IntlMessageParametersExtractor(),
193198
new TypeScriptMessageParametersPrinter(),
194199
new Filesystem(),
195200
);
196-
$translationsDumper->addExcludedDomain('foobar');
197-
$translationsDumper->addIncludedDomain('messages');
201+
$translationsDumper->dump(
202+
catalogues: self::getMessageCatalogues(),
203+
dumpDir: self::$translationsDumpDir,
204+
includedDomains: ['messages'],
205+
excludedDomains: ['foobar'],
206+
);
198207
}
199208

200209
/**

0 commit comments

Comments
 (0)