feat(compiler): move translation service initialization to see the logs#1705
Conversation
| `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. |
There was a problem hiding this comment.
That's what is solved in the PR (beside e2e tests and logs clenaup)
| if (!this.translationService) { | ||
| throw new Error("Translation service not initialized"); | ||
| } |
There was a problem hiding this comment.
Quite dumb check, we make sure TranslationService is never null in constructor, since it's not something heavy we should delay creating
| const delay = this.config?.delayMedian ?? 0; | ||
| const actualDelay = this.getRandomDelay(delay); | ||
|
|
||
| this.logger.debug( |
There was a problem hiding this comment.
These were mostly added by LLM for debugging, we can add them back with LLM if needed
91c0bef to
85ceb48
Compare
We don't really need to initialize it inside the server, since server is only a way to use the service in the dev mode. Logs of the server are written to a file due to it being started from process which doesn't have access to console. But we want to see TranslationService initialization logs in the console, and the clearest way is to start it early, rather than extracting validation into a separate function.
85ceb48 to
bed9c1f
Compare
| if: github.event.pull_request.user.login != 'dependabot[bot]' | ||
| run: pnpm changeset status --since origin/main | ||
|
|
||
| compiler-e2e: |
There was a problem hiding this comment.
Why this is a separate job:
- this test should probably be optional for now
- it's easier to see which tests failed exactly, e2e or the usual ones
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.
| "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", | ||
| "test:e2e": "playwright test", |
There was a problem hiding this comment.
Reporter is set in the config
| "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", |
There was a problem hiding this comment.
Build is required on turbo side
| - name: Install dependencies | ||
| run: pnpm install | ||
|
|
||
| - name: Install Playwright Browsers |
There was a problem hiding this comment.
Playwright says caching browsers isn't worth it - https://playwright.dev/docs/ci#caching-browsers (and we use only one)
|
|
||
| logger.info(`🌍 Build mode: ${buildMode}`); | ||
|
|
||
| if (metadataFilePath) { |
There was a problem hiding this comment.
This was internal info
There was a problem hiding this comment.
Pull request overview
This PR refactors the translation service initialization to display logs in the console instead of a log file, and adds e2e test infrastructure for the compiler package. The key motivation is to make TranslationService initialization errors visible during development since the server process doesn't have console access.
Key Changes:
- Moved translator initialization logic from
translator-factory.tsinto theTranslationServiceconstructor - Renamed
Serviceclass toLingoTranslatorfor clarity - Updated test script naming from
test:preparetotest:e2e:preparethroughout the codebase - Added CI workflow for running e2e tests with Playwright
- Changed many logging calls from
infotodebuglevel to reduce console noise
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
packages/new-compiler/src/translators/translation-service.ts |
Integrated translator and cache creation directly into TranslationService constructor; removed factory dependency |
packages/new-compiler/src/translators/translator-factory.ts |
Removed entire file as its logic was moved into TranslationService |
packages/new-compiler/src/translators/memory-cache.ts |
New in-memory cache implementation for use with pseudotranslator |
packages/new-compiler/src/translators/lingo/translator.ts |
Renamed Service class to LingoTranslator; reformatted imports |
packages/new-compiler/src/translators/lingo/provider-details.ts |
Removed unused helper functions; simplified error messages |
packages/new-compiler/src/translators/lingo/model-factory.ts |
Updated to use new error formatting; added TODO comment |
packages/new-compiler/src/translators/pluralization/service.ts |
Changed logging levels from info to debug; refactored cache check logic; removed unused methods |
packages/new-compiler/src/translators/pseudotranslator/index.ts |
Removed verbose trace-level debug logs |
packages/new-compiler/src/translation-server/translation-server.ts |
Accept optional TranslationService in constructor; read startPort from config instead of separate parameter |
packages/new-compiler/src/plugin/unplugin.ts |
Create TranslationService before passing to server |
packages/new-compiler/src/plugin/next.ts |
Create TranslationService before passing to server; improved error logging |
packages/new-compiler/src/plugin/build-translator.ts |
Create TranslationService before passing to server; improved error logging |
packages/new-compiler/package.json |
Renamed test scripts from test:prepare to test:e2e:prepare; removed --reporter=list flags |
turbo.json |
Added test:e2e:prepare and test:e2e tasks with proper dependencies |
.github/workflows/pr-check.yml |
Added new compiler-e2e job to run Playwright tests in CI |
packages/new-compiler/tests/**/*.md |
Updated documentation to use new test:e2e:prepare script name |
packages/new-compiler/tests/helpers/**/*.ts |
Updated error messages to reference new script name |
demo/new-compiler-next16/package.json |
Changed workspace dependency specifier from ^1.0.0-beta to * |
pnpm-lock.yaml |
Lockfile updates reflecting dependency changes |
.changeset/sharp-yaks-cough.md |
Added changeset documenting the patch |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/new-compiler/src/translators/lingo/translator.ts:7
- The import formatting change that combines multiple imports into a single line reduces code readability. The original multi-line format was more readable and easier to maintain. Consider reverting to the multi-line import format for better code clarity.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return hashes.reduce( | ||
| (acc, hash) => ({ ...acc, [hash]: localeCache.get(hash) }), | ||
| {}, | ||
| ); |
There was a problem hiding this comment.
The reduce operation on line 21-23 has a potential bug. When a hash is not found in localeCache, localeCache.get(hash) returns undefined, which will be set as the value for that hash in the accumulator. This means the returned object will contain entries with undefined values for missing hashes. Consider filtering out undefined values or only including hashes that exist in the cache.
| return hashes.reduce( | |
| (acc, hash) => ({ ...acc, [hash]: localeCache.get(hash) }), | |
| {}, | |
| ); | |
| return hashes.reduce((acc, hash) => { | |
| const value = localeCache.get(hash); | |
| if (value !== undefined) { | |
| acc[hash] = value; | |
| } | |
| return acc; | |
| }, {} as Record<string, string>); |
| constructor() {} | ||
|
|
There was a problem hiding this comment.
The empty constructor serves no purpose and can be removed. TypeScript doesn't require an explicit constructor if there's no initialization logic.
| constructor() {} |
| this.logger.warn(`⚠️ Translation setup error: \n${errorMsg}\n | ||
| ⚠️ Auto-fallback to pseudotranslator in development mode. | ||
| Set the required API keys for real translations.`); |
There was a problem hiding this comment.
The multi-line string literal uses a backtick template with embedded newlines, but this creates a string with leading whitespace on lines 121-122. The indentation in the source code will be included in the output message, which may look incorrect when logged. Consider using a dedented template literal or joining separate strings.
| this.logger.warn(`⚠️ Translation setup error: \n${errorMsg}\n | |
| ⚠️ Auto-fallback to pseudotranslator in development mode. | |
| Set the required API keys for real translations.`); | |
| this.logger.warn( | |
| `⚠️ Translation setup error: \n${errorMsg}\n` + | |
| "⚠️ Auto-fallback to pseudotranslator in development mode.\n" + | |
| "Set the required API keys for real translations.", | |
| ); |
…gs (lingodotdev#1705) * feat: move translation service initialization to see the logs We don't really need to initialize it inside the server, since server is only a way to use the service in the dev mode. Logs of the server are written to a file due to it being started from process which doesn't have access to console. But we want to see TranslationService initialization logs in the console, and the clearest way is to start it early, rather than extracting validation into a separate function. * fix: new ai sdk api compatibility * chore: add changeset * feat: add compiler e2e tests * fix: run tests preparation through turbo * fix: limit installed browsers to chromium and fix trubo config * fix: run build before tests * fix: get rid of noisy logs * fix: cleanup logs and unused functions * fix: compiler version in next demo --------- Co-authored-by: Max Prilutskiy <5614659+maxprilutskiy@users.noreply.github.com>
Summary
We don't really need to initialize it inside the server, since server is only a way to use the service in the dev mode.
Logs of the server are written to a file due to it being started from process which doesn't have access to console. But we want to see TranslationService initialization logs in the console, and the clearest way is to start it early, rather than extracting validation into a separate function.
Changes
Checklist