-
Notifications
You must be signed in to change notification settings - Fork 664
feat(compiler): move translation service initialization to see the logs #1705
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: main
Are you sure you want to change the base?
Changes from all commits
c225794
861ca01
bed9c1f
6aa0e9a
fce5c3a
790182e
55431fe
fb8ca8c
196b8f8
688a1c5
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@lingo.dev/compiler": patch | ||
| --- | ||
|
|
||
| Show logs of the translator initialization to notify about possible problems with LLM keys |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -71,3 +71,47 @@ jobs: | |
| - name: Require changeset to be present in PR | ||
| if: github.event.pull_request.user.login != 'dependabot[bot]' | ||
| run: pnpm changeset status --since origin/main | ||
|
|
||
| compiler-e2e: | ||
| needs: check | ||
| timeout-minutes: 60 | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| ref: ${{github.event.pull_request.head.sha}} | ||
| fetch-depth: 0 | ||
|
|
||
| - name: Use Node.js | ||
| uses: actions/setup-node@v6 | ||
| with: | ||
| node-version: "22" | ||
|
|
||
| - name: Install pnpm | ||
| uses: pnpm/action-setup@v4 | ||
| with: | ||
| version: 9.12.3 | ||
|
|
||
| - name: Configure pnpm cache | ||
| id: pnpm-cache | ||
| run: echo "STORE_PATH=$(pnpm store path)" >> $GITHUB_OUTPUT | ||
| - uses: actions/cache@v3 | ||
| with: | ||
| path: ${{ steps.pnpm-cache.outputs.STORE_PATH }} | ||
| key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-pnpm-store- | ||
|
|
||
| - name: Install dependencies | ||
| run: pnpm install | ||
|
|
||
| - name: Install Playwright Browsers | ||
|
Collaborator
Author
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. Playwright says caching browsers isn't worth it - https://playwright.dev/docs/ci#caching-browsers (and we use only one) |
||
| run: pnpm exec playwright install chromium --with-deps | ||
| working-directory: packages/new-compiler | ||
|
|
||
| - name: Configure Turbo cache | ||
| uses: dtinth/setup-github-actions-caching-for-turbo@v1 | ||
|
|
||
| - name: Run E2E tests | ||
| run: pnpm turbo run test:e2e --filter=./packages/new-compiler | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| { | ||
| "name": "@lingo.dev/compiler", | ||
| "version": "0.1.1", | ||
| "version": "0.1.2", | ||
| "description": "Lingo.dev Compiler", | ||
| "private": false, | ||
| "repository": { | ||
|
|
@@ -123,11 +123,11 @@ | |
| "clean": "rm -rf build", | ||
| "test": "vitest --run", | ||
| "test:watch": "vitest -w", | ||
| "test:prepare": "pnpm build && tsx tests/helpers/prepare-fixtures.ts", | ||
| "test:e2e": "playwright test --reporter=list", | ||
| "test:e2e:next": "playwright test --grep next --reporter=list", | ||
| "test:e2e:vite": "playwright test --grep vite --reporter=list", | ||
| "test:e2e:shared": "playwright test tests/e2e/shared --reporter=list", | ||
| "test:e2e:prepare": "tsx tests/helpers/prepare-fixtures.ts", | ||
|
Collaborator
Author
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. Build is required on turbo side |
||
| "test:e2e": "playwright test", | ||
|
Collaborator
Author
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. Reporter is set in the config |
||
| "test:e2e:next": "playwright test --grep next", | ||
| "test:e2e:vite": "playwright test --grep vite", | ||
| "test:e2e:shared": "playwright test tests/e2e/shared", | ||
| "test:e2e:headed": "playwright test --headed", | ||
| "test:e2e:ui": "playwright test --ui", | ||
| "test:e2e:debug": "playwright test --debug", | ||
|
|
@@ -186,4 +186,4 @@ | |
| "optional": true | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,12 +11,9 @@ import fs from "fs/promises"; | |
| import path from "path"; | ||
| import type { LingoConfig, MetadataSchema } from "../types"; | ||
| import { logger } from "../utils/logger"; | ||
| import { | ||
| startTranslationServer, | ||
| type TranslationServer, | ||
| } from "../translation-server"; | ||
| import { startTranslationServer, type TranslationServer, } from "../translation-server"; | ||
| import { loadMetadata } from "../metadata/manager"; | ||
| import { createCache, type TranslationCache } from "../translators"; | ||
| import { createCache, type TranslationCache, TranslationService, } from "../translators"; | ||
| import { dictionaryFrom } from "../translators/api"; | ||
| import type { LocaleCode } from "lingo.dev/spec"; | ||
|
|
||
|
|
@@ -67,10 +64,6 @@ export async function processBuildTranslations( | |
|
|
||
| logger.info(`🌍 Build mode: ${buildMode}`); | ||
|
|
||
| if (metadataFilePath) { | ||
|
Collaborator
Author
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. This was internal info |
||
| logger.info(`📋 Using build metadata file: ${metadataFilePath}`); | ||
| } | ||
|
|
||
| const metadata = await loadMetadata(metadataFilePath); | ||
|
|
||
| if (!metadata || Object.keys(metadata.entries).length === 0) { | ||
|
|
@@ -108,7 +101,7 @@ export async function processBuildTranslations( | |
|
|
||
| try { | ||
| translationServer = await startTranslationServer({ | ||
| startPort: config.dev.translationServerStartPort, | ||
| translationService: new TranslationService(config, logger), | ||
| onError: (err) => { | ||
| logger.error("Translation server error:", err); | ||
| }, | ||
|
|
@@ -175,7 +168,10 @@ export async function processBuildTranslations( | |
| stats, | ||
| }; | ||
| } catch (error) { | ||
| logger.error("❌ Translation generation failed:", error); | ||
| logger.error( | ||
| "❌ Translation generation failed:\n", | ||
| error instanceof Error ? error.message : error, | ||
| ); | ||
| process.exit(1); | ||
| } finally { | ||
| if (translationServer) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,7 @@ import { startOrGetTranslationServer } from "../translation-server/translation-s | |
| import { cleanupExistingMetadata, getMetadataPath } from "../metadata/manager"; | ||
| import { registerCleanupOnCurrentProcess } from "./cleanup"; | ||
| import { useI18nRegex } from "./transform/use-i18n"; | ||
| import { TranslationService } from "../translators"; | ||
|
|
||
| export type LingoNextPluginOptions = PartialLingoConfig; | ||
|
|
||
|
|
@@ -205,14 +206,12 @@ export async function withLingo( | |
| `Initializing Lingo.dev compiler. Is dev mode: ${isDev}. Is main runner: ${isMainRunner()}`, | ||
| ); | ||
|
|
||
| // TODO (AleksandrSl 12/12/2025): Add API keys validation too, so we can log it nicely. | ||
|
Collaborator
Author
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. That's what is solved in the PR (beside e2e tests and logs clenaup) |
||
|
|
||
| // Try to start up the translation server once. | ||
| // We have two barriers, a simple one here and a more complex one inside the startTranslationServer which doesn't start the server if it can find one running. | ||
| // We do not use isMainRunner here, because we need to start the server as early as possible, so the loaders get the translation server url. The main runner in dev mode runs after a dev server process is started. | ||
| if (isDev && !process.env.LINGO_TRANSLATION_SERVER_URL) { | ||
| const translationServer = await startOrGetTranslationServer({ | ||
| startPort: lingoConfig.dev.translationServerStartPort, | ||
| translationService: new TranslationService(lingoConfig, logger), | ||
| onError: (err) => { | ||
| logger.error("Translation server error:", err); | ||
| }, | ||
|
|
@@ -298,7 +297,6 @@ export async function withLingo( | |
| } | ||
|
|
||
| logger.info("Running post-build translation generation..."); | ||
| logger.info(`Build mode: Using metadata file: ${metadataFilePath}`); | ||
|
|
||
| try { | ||
| await processBuildTranslations({ | ||
|
|
@@ -307,7 +305,10 @@ export async function withLingo( | |
| metadataFilePath, | ||
| }); | ||
| } catch (error) { | ||
| logger.error("Translation generation failed:", error); | ||
| logger.error( | ||
| "Translation generation failed:", | ||
| error instanceof Error ? error.message : error, | ||
| ); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,11 +17,7 @@ import { URL } from "url"; | |
| import { WebSocket, WebSocketServer } from "ws"; | ||
| import type { MetadataSchema, TranslationMiddlewareConfig } from "../types"; | ||
| import { getLogger } from "./logger"; | ||
| import { | ||
| createCache, | ||
| createTranslator, | ||
| TranslationService, | ||
| } from "../translators"; | ||
| import { TranslationService } from "../translators"; | ||
| import { | ||
| createEmptyMetadata, | ||
| getMetadataPath, | ||
|
|
@@ -33,38 +29,21 @@ import type { LocaleCode } from "lingo.dev/spec"; | |
| import { parseLocaleOrThrow } from "../utils/is-valid-locale"; | ||
|
|
||
| export interface TranslationServerOptions { | ||
| /** | ||
| * Starting port to try (will find next available if taken) | ||
| * @default 3456 | ||
| */ | ||
| startPort?: number; | ||
|
|
||
| /** | ||
| * Configuration for translation generation | ||
| */ | ||
| config: TranslationMiddlewareConfig; | ||
|
|
||
| /** | ||
| * Callback when server is ready | ||
| */ | ||
| translationService?: TranslationService; | ||
| onReady?: (port: number) => void; | ||
|
|
||
| /** | ||
| * Callback on error | ||
| */ | ||
| onError?: (error: Error) => void; | ||
| } | ||
|
|
||
| export class TranslationServer { | ||
| private server: http.Server | null = null; | ||
| private url: string | undefined = undefined; | ||
| private logger; | ||
| private config: TranslationMiddlewareConfig; | ||
| private configHash: string; | ||
| private startPort: number; | ||
| private onReadyCallback?: (port: number) => void; | ||
| private onErrorCallback?: (error: Error) => void; | ||
| private translationService: TranslationService | null = null; | ||
| private readonly config: TranslationMiddlewareConfig; | ||
| private readonly configHash: string; | ||
| private readonly startPort: number; | ||
| private readonly onReadyCallback?: (port: number) => void; | ||
| private readonly onErrorCallback?: (error: Error) => void; | ||
| private metadata: MetadataSchema | null = null; | ||
| private connections: Set<Socket> = new Set(); | ||
| private wss: WebSocketServer | null = null; | ||
|
|
@@ -75,11 +54,16 @@ export class TranslationServer { | |
| private isBusy = false; | ||
| private busyTimeout: NodeJS.Timeout | null = null; | ||
| private readonly BUSY_DEBOUNCE_MS = 500; // Time after last translation to send "idle" event | ||
| private readonly translationService: TranslationService; | ||
|
|
||
| constructor(options: TranslationServerOptions) { | ||
| this.config = options.config; | ||
| this.configHash = hashConfig(options.config); | ||
| this.startPort = options.startPort || 60000; | ||
| this.translationService = | ||
| options.translationService ?? | ||
| // Fallback is for CLI start only. | ||
| new TranslationService(options.config, getLogger(options.config)); | ||
| this.startPort = options.config.dev.translationServerStartPort; | ||
| this.onReadyCallback = options.onReady; | ||
| this.onErrorCallback = options.onError; | ||
| this.logger = getLogger(this.config); | ||
|
|
@@ -95,19 +79,6 @@ export class TranslationServer { | |
|
|
||
| this.logger.info(`🔧 Initializing translator...`); | ||
|
|
||
| const translator = createTranslator(this.config, this.logger); | ||
| const cache = createCache(this.config); | ||
|
|
||
| this.translationService = new TranslationService( | ||
| translator, | ||
| cache, | ||
| { | ||
| sourceLocale: this.config.sourceLocale, | ||
| pluralization: this.config.pluralization, | ||
| }, | ||
| this.logger, | ||
| ); | ||
|
|
||
| const port = await this.findAvailablePort(this.startPort); | ||
|
|
||
| return new Promise((resolve, reject) => { | ||
|
|
@@ -281,14 +252,13 @@ export class TranslationServer { | |
| * Start a new server or get the URL of an existing one on the preferred port. | ||
| * | ||
| * This method optimizes for the common case where a translation server is already | ||
| * running on port 60000. If that port is taken, it checks if it's our service | ||
| * running on a preferred port. If that port is taken, it checks if it's our service | ||
| * by calling the health check endpoint. If it is, we reuse it instead of starting | ||
| * a new server on a different port. | ||
| * | ||
| * @returns URL of the running server (new or existing) | ||
| */ | ||
| async startOrGetUrl(): Promise<string> { | ||
| // If this instance already has a server running, return its URL | ||
| if (this.server && this.url) { | ||
| this.logger.info(`Using existing server instance at ${this.url}`); | ||
| return this.url; | ||
|
|
@@ -527,7 +497,6 @@ export class TranslationServer { | |
|
|
||
| res.on("end", () => { | ||
| try { | ||
| // Check if response is valid and has the expected structure | ||
| if (res.statusCode === 200) { | ||
| const json = JSON.parse(data); | ||
| // Our translation server returns { status: "ok", port: ..., configHash: ... } | ||
|
|
@@ -680,11 +649,6 @@ export class TranslationServer { | |
| ); | ||
| return; | ||
| } | ||
|
|
||
| if (!this.translationService) { | ||
| throw new Error("Translation service not initialized"); | ||
| } | ||
|
|
||
| // Reload metadata to ensure we have the latest entries | ||
| // (new entries may have been added since server started) | ||
| await this.reloadMetadata(); | ||
|
|
@@ -747,10 +711,6 @@ export class TranslationServer { | |
| try { | ||
| const parsedLocale = parseLocaleOrThrow(locale); | ||
|
|
||
| if (!this.translationService) { | ||
| throw new Error("Translation service not initialized"); | ||
| } | ||
|
Comment on lines
-750
to
-752
Collaborator
Author
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. Quite dumb check, we make sure |
||
|
|
||
| // Reload metadata to ensure we have the latest entries | ||
| // (new entries may have been added since server started) | ||
| await this.reloadMetadata(); | ||
|
|
@@ -842,9 +802,6 @@ export function hashConfig(config: Record<string, SerializableValue>): string { | |
| return crypto.createHash("md5").update(serialized).digest("hex").slice(0, 12); | ||
| } | ||
|
|
||
| /** | ||
| * Create and start a translation server | ||
| */ | ||
| export async function startTranslationServer( | ||
| options: TranslationServerOptions, | ||
| ): Promise<TranslationServer> { | ||
|
|
@@ -856,10 +813,10 @@ export async function startTranslationServer( | |
| /** | ||
| * Create a translation server and start it or reuse an existing one on the preferred port | ||
| * | ||
| * Since we have little control over the dev server start in next, we can start the translation server only in the loader, | ||
| * and loaders could be started from multiple processes (it seems) or similar we need a way to avoid starting multiple servers. | ||
| * Since we have little control over the dev server start in next, we can start the translation server only in the async config or in the loader, | ||
| * they both could be run in different processes, and we need a way to avoid starting multiple servers. | ||
| * This one will try to start a server on the preferred port (which seems to be an atomic operation), and if it fails, | ||
| * it checks if the server already started is ours and returns its url. | ||
| * it checks if the server that is already started is ours and returns its url. | ||
| * | ||
| * @returns Object containing the server instance and its URL | ||
| */ | ||
|
|
||
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.
Why this is a separate job:
It needs the previous check to pass because if usual tests fail, I don't think it makes a lot of sense to run e2e.