-
Notifications
You must be signed in to change notification settings - Fork 20
IBX-12091: Added TypeScript translation extraction support #1967
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
base: 6.0
Are you sure you want to change the base?
Changes from all commits
a6c9b07
8a5efda
f1532dc
2d44f1c
8b44786
8b9bbb0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -84,6 +84,13 @@ | |||||
| satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} | ||||||
| satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} | ||||||
|
|
||||||
| - uses: actions/setup-node@v4 | ||||||
| with: | ||||||
| node-version: '24' | ||||||
|
|
||||||
| - name: Install frontend dependencies | ||||||
| run: yarn install | ||||||
|
Check warning on line 92 in .github/workflows/backend-ci.yaml
|
||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. SonarQube:
This looks relevant to the case, but if it's false positive, please mark it as false positive. I'm not able to assess this one on my own. (you can use GitHub to log in to Sonar, so you can access full report, lmk if you have questions) |
||||||
|
|
||||||
| - name: Setup problem matchers for PHPUnit | ||||||
| run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" | ||||||
|
|
||||||
|
|
@@ -131,6 +138,13 @@ | |||||
| satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} | ||||||
| satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} | ||||||
|
|
||||||
| - uses: actions/setup-node@v4 | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| with: | ||||||
| node-version: '24' | ||||||
|
|
||||||
| - name: Install frontend dependencies | ||||||
| run: yarn install | ||||||
|
Check warning on line 146 in .github/workflows/backend-ci.yaml
|
||||||
|
|
||||||
| - name: Setup problem matchers for PHPUnit | ||||||
| run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" | ||||||
|
|
||||||
|
|
@@ -182,6 +196,13 @@ | |||||
| satis-network-key: ${{ secrets.SATIS_NETWORK_KEY }} | ||||||
| satis-network-token: ${{ secrets.SATIS_NETWORK_TOKEN }} | ||||||
|
|
||||||
| - uses: actions/setup-node@v4 | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| with: | ||||||
| node-version: '24' | ||||||
|
|
||||||
| - name: Install frontend dependencies | ||||||
| run: yarn install | ||||||
|
Check warning on line 204 in .github/workflows/backend-ci.yaml
|
||||||
|
|
||||||
| - name: Setup problem matchers for PHPUnit | ||||||
| run: echo "::add-matcher::${{ runner.tool_cache }}/phpunit.json" | ||||||
|
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,193 @@ | ||||||
| <?php | ||||||
|
|
||||||
| /** | ||||||
| * @copyright Copyright (C) Ibexa AS. All rights reserved. | ||||||
| * @license For full copyright and license information view LICENSE file distributed with this source code. | ||||||
| */ | ||||||
| declare(strict_types=1); | ||||||
|
|
||||||
| namespace Ibexa\AdminUi\Translation\Extractor; | ||||||
|
|
||||||
| use JsonException; | ||||||
| use RuntimeException; | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is run in CLI, then one of Symfony\Console\ exceptions should be used. Comes from SonarQube analysis, but I find this valid. |
||||||
| use Symfony\Component\Process\InputStream; | ||||||
| use Symfony\Component\Process\Process; | ||||||
|
|
||||||
| final class TypeScriptExtractorClient | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| { | ||||||
| private const DEFAULT_RESPONSE_TIMEOUT_SECONDS = 30.0; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| private string $parserScriptPath; | ||||||
|
|
||||||
| private bool $isRuntimeReady = false; | ||||||
|
|
||||||
| private ?Process $serverProcess = null; | ||||||
|
|
||||||
| private ?InputStream $serverInput = null; | ||||||
|
|
||||||
| private string $responseBuffer = ''; | ||||||
|
|
||||||
| public function __construct( | ||||||
| ?string $parserScriptPath = null, | ||||||
| private readonly string $nodeBinary = 'node', | ||||||
| private readonly float $responseTimeoutSeconds = self::DEFAULT_RESPONSE_TIMEOUT_SECONDS, | ||||||
| ) { | ||||||
| $this->parserScriptPath = $parserScriptPath ?? __DIR__ . '/typescript_translation_extractor.mjs'; | ||||||
| } | ||||||
|
|
||||||
| public function __destruct() | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this have a return type? |
||||||
| { | ||||||
| $this->discardServerProcess(); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * @return array{messages?: mixed, warnings?: mixed, error?: string} | ||||||
| */ | ||||||
| public function extract(string $filePath): array | ||||||
| { | ||||||
| $this->assertRuntimeIsReady(); | ||||||
| $this->ensureServerStarted(); | ||||||
|
|
||||||
| $this->getServerInput()->write($filePath . "\n"); | ||||||
|
|
||||||
| return $this->readExtractionResponse(); | ||||||
| } | ||||||
|
|
||||||
| private function assertRuntimeIsReady(): void | ||||||
| { | ||||||
| if ($this->isRuntimeReady) { | ||||||
| return; | ||||||
| } | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Architecture: is there a better way of checking readiness? For e.g., existence of some file?
I don't have a solution, because I don't know what this runtime does, but maybe it's worth looking into. |
||||||
|
|
||||||
| if (!is_file($this->parserScriptPath)) { | ||||||
| throw new RuntimeException(sprintf( | ||||||
|
Check warning on line 63 in src/lib/Translation/Extractor/TypeScriptExtractorClient.php
|
||||||
| 'TypeScript translation extractor script not found: %s.', | ||||||
| $this->parserScriptPath, | ||||||
| )); | ||||||
| } | ||||||
|
|
||||||
| $runtimeCheckProcess = new Process([ | ||||||
| $this->nodeBinary, | ||||||
| $this->parserScriptPath, | ||||||
| '--check-runtime', | ||||||
| ]); | ||||||
| $runtimeCheckProcess->run(); | ||||||
|
|
||||||
| if (!$runtimeCheckProcess->isSuccessful()) { | ||||||
| throw new RuntimeException(sprintf( | ||||||
|
Check warning on line 77 in src/lib/Translation/Extractor/TypeScriptExtractorClient.php
|
||||||
| 'TypeScript translation extractor runtime is not available. Please ensure `%s` is installed and `@typescript-eslint/typescript-estree` is available for the admin-ui package. %s', | ||||||
| $this->nodeBinary, | ||||||
| trim($runtimeCheckProcess->getErrorOutput()), | ||||||
| )); | ||||||
| } | ||||||
|
|
||||||
| $this->isRuntimeReady = true; | ||||||
| } | ||||||
|
|
||||||
| private function ensureServerStarted(): void | ||||||
| { | ||||||
| if ($this->serverProcess !== null && $this->serverProcess->isRunning()) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| $processInput = new InputStream(); | ||||||
|
|
||||||
| $extractorProcess = new Process([ | ||||||
| $this->nodeBinary, | ||||||
| $this->parserScriptPath, | ||||||
| '--serve', | ||||||
| ]); | ||||||
| $extractorProcess->setInput($processInput); | ||||||
| $extractorProcess->setTimeout(null); | ||||||
| $extractorProcess->start(); | ||||||
|
|
||||||
| $this->serverInput = $processInput; | ||||||
| $this->serverProcess = $extractorProcess; | ||||||
| $this->responseBuffer = ''; | ||||||
| } | ||||||
|
|
||||||
| private function getServerInput(): InputStream | ||||||
| { | ||||||
| if ($this->serverInput === null) { | ||||||
| throw new RuntimeException('TypeScript translation extractor input stream has not been started.'); | ||||||
|
Check warning on line 112 in src/lib/Translation/Extractor/TypeScriptExtractorClient.php
|
||||||
| } | ||||||
|
|
||||||
| return $this->serverInput; | ||||||
| } | ||||||
|
|
||||||
| private function getServerProcess(): Process | ||||||
| { | ||||||
| if ($this->serverProcess === null) { | ||||||
| throw new RuntimeException('TypeScript translation extractor process has not been started.'); | ||||||
|
Check warning on line 121 in src/lib/Translation/Extractor/TypeScriptExtractorClient.php
|
||||||
| } | ||||||
|
|
||||||
| return $this->serverProcess; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * @return array{messages?: mixed, warnings?: mixed, error?: string} | ||||||
| */ | ||||||
| private function readExtractionResponse(): array | ||||||
| { | ||||||
| $process = $this->getServerProcess(); | ||||||
| $responseDeadline = microtime(true) + $this->responseTimeoutSeconds; | ||||||
|
|
||||||
| while (!str_contains($this->responseBuffer, "\n")) { | ||||||
| if (!$process->isRunning()) { | ||||||
| $this->discardServerProcess(); | ||||||
|
|
||||||
| throw new RuntimeException(sprintf( | ||||||
|
Check warning on line 139 in src/lib/Translation/Extractor/TypeScriptExtractorClient.php
|
||||||
| 'TypeScript translation extractor process terminated unexpectedly: %s', | ||||||
| trim($process->getErrorOutput()), | ||||||
| )); | ||||||
| } | ||||||
|
|
||||||
| if (microtime(true) > $responseDeadline) { | ||||||
| $this->discardServerProcess(); | ||||||
|
|
||||||
| throw new RuntimeException( | ||||||
|
Check warning on line 148 in src/lib/Translation/Extractor/TypeScriptExtractorClient.php
|
||||||
| 'Timed out waiting for the TypeScript translation extractor process to respond.', | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| $this->responseBuffer .= $process->getIncrementalOutput(); | ||||||
|
|
||||||
| if (!str_contains($this->responseBuffer, "\n")) { | ||||||
| usleep(2000); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| [$responseLine, $rest] = explode("\n", $this->responseBuffer, 2); | ||||||
| $this->responseBuffer = $rest; | ||||||
|
|
||||||
| return $this->decodeResponse($responseLine); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * @return array{messages?: mixed, warnings?: mixed, error?: string} | ||||||
| */ | ||||||
| private function decodeResponse(string $responseLine): array | ||||||
| { | ||||||
| try { | ||||||
| $responseData = json_decode($responseLine, true, 512, JSON_THROW_ON_ERROR); | ||||||
| } catch (JsonException) { | ||||||
| return ['error' => 'Unable to decode extractor output.']; | ||||||
| } | ||||||
|
|
||||||
| if (!is_array($responseData)) { | ||||||
| return ['error' => 'Extractor output must be a JSON object.']; | ||||||
| } | ||||||
|
|
||||||
| return $responseData; | ||||||
| } | ||||||
|
|
||||||
| private function discardServerProcess(): void | ||||||
| { | ||||||
| $this->serverInput?->close(); | ||||||
| $this->serverProcess?->stop(3); | ||||||
|
|
||||||
| $this->serverProcess = null; | ||||||
| $this->serverInput = null; | ||||||
| $this->responseBuffer = ''; | ||||||
| } | ||||||
| } | ||||||
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.
outdated action version