fix(transloco): grammar fix misspelled scope hint error message#904
Open
focbenz wants to merge 2 commits intojsverse:masterfrom
Open
fix(transloco): grammar fix misspelled scope hint error message#904focbenz wants to merge 2 commits intojsverse:masterfrom
focbenz wants to merge 2 commits intojsverse:masterfrom
Conversation
@jsverse/transloco
@jsverse/transloco-locale
@jsverse/transloco-messageformat
@jsverse/transloco-optimize
@jsverse/transloco-persist-lang
@jsverse/transloco-persist-translations
@jsverse/transloco-preload-langs
@jsverse/transloco-schematics
@jsverse/transloco-scoped-libs
@jsverse/transloco-utils
@jsverse/transloco-validator
commit: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="libs/transloco/src/lib/tests/service/fallbacks.spec.ts">
<violation number="1" location="libs/transloco/src/lib/tests/service/fallbacks.spec.ts:156">
P0: The test asserts the old, ungrammatical string `'did you misspelled the scope name'`, but the source code was fixed to `'did you misspell the scope name?'`. This test will always fail.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
📝 WalkthroughWalkthroughAdds a unit test verifying behavior when a scoped translation and its fallbacks all fail, and corrects a grammatical typo in the fallback-failure error message. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/transloco/src/lib/tests/service/fallbacks.spec.ts`:
- Around line 155-157: The test assertion in the catchError block checks for the
outdated message 'did you misspelled the scope name' — update the expectation in
the test (the expect(...).toContain call inside the catchError callback in
fallbacks.spec.ts) to match the corrected service message 'did you misspell the
scope name?' so the test compares against the exact new text used by the
transloco service.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2bd724e-ac45-44ab-b7c3-1ae068e44b9c
📒 Files selected for processing (2)
libs/transloco/src/lib/tests/service/fallbacks.spec.tslibs/transloco/src/lib/transloco.service.ts
Co-authored-by: cubic-dev-ai[bot]
There was a problem hiding this comment.
🧹 Nitpick comments (1)
libs/transloco/src/lib/tests/service/fallbacks.spec.ts (1)
141-164: Consider adding verification that the error was actually caught.The test relies on
catchErrorbeing triggered, but if the error path isn't reached (e.g., due to unexpected loader behavior), the test would pass without any assertion executing. The similar test at lines 111-139 includes an additional assertion outsidecatchError(expect(service.load).toHaveBeenCalledTimes(2)).♻️ Suggested improvement
it(`GIVEN a scoped lang that fails to load AND all fallbacks fail THEN the error should contain a scope misspelling hint`, fakeAsync(() => { const service = createService( { prodMode: false, // Show error messages in thrown errors fallbackLang: 'fallbackNotExists', failedRetries: 0, }, { loader }, ); + spyOn(service, 'load').and.callThrough(); + let errorCaught = false; service .load('admin/notExists') .pipe( catchError((err) => { + errorCaught = true; expect(err.message).toContain('did you misspell the scope name'); return of(''); }), ) .subscribe(); // admin/notExists will try 1 time then fallback 1 time runLoader(2); + expect(errorCaught).toBe(true); + expect(service.load).toHaveBeenCalledTimes(2); }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/transloco/src/lib/tests/service/fallbacks.spec.ts` around lines 141 - 164, The test may pass without verifying the error path executed; ensure the catchError branch actually ran by adding an assertion outside the catchError (e.g., expect(loader.get).toHaveBeenCalledTimes(2) or increment a local flag inside catchError and assert the flag is 1 after runLoader) so you explicitly verify service.load('admin/notExists') attempted loading/fallbacks; reference the existing symbols service.load, loader (or loader.get), and runLoader to locate where to add the assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@libs/transloco/src/lib/tests/service/fallbacks.spec.ts`:
- Around line 141-164: The test may pass without verifying the error path
executed; ensure the catchError branch actually ran by adding an assertion
outside the catchError (e.g., expect(loader.get).toHaveBeenCalledTimes(2) or
increment a local flag inside catchError and assert the flag is 1 after
runLoader) so you explicitly verify service.load('admin/notExists') attempted
loading/fallbacks; reference the existing symbols service.load, loader (or
loader.get), and runLoader to locate where to add the assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4221894d-d09a-4709-8174-5220c6148287
📒 Files selected for processing (1)
libs/transloco/src/lib/tests/service/fallbacks.spec.ts
focbenz
force-pushed
the
typo-fix-scope-misspelled-hint
branch
from
397b0d1 to
06bc3c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/transloco/src/lib/tests/service/fallbacks.spec.ts`:
- Around line 141-163: The loader mock used by the test for the spec that calls
service.load('admin/notExists') must be made to throw for both scoped paths so
the error branch is exercised; update the loader's error list (used by the mock
referenced in this test) to include 'admin/notExists' and
'admin/fallbackNotExists' (in addition to the existing 'notExists',
'notExists2', 'fallbackNotExists'), so that createService(...) loading
'admin/notExists' with fallbackLang 'fallbackNotExists' triggers errors and
causes the catchError assertion to run when runLoader(2) is called.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 650ee9a1-7656-4926-a461-19c96af109e3
📒 Files selected for processing (1)
libs/transloco/src/lib/tests/service/fallbacks.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Error message displayed when a scoped lang fails to load AND all fallbacks fail:
Unable to load translation and all the fallback languages, did you misspelled the scope name?VVV
Unable to load translation and all the fallback languages, did you misspell the scope name?PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Error message displayed when a scoped lang fails to load AND all fallbacks fail:
Unable to load translation and all the fallback languages, did you misspelled the scope name?Issue Number: N/A
What is the new behavior?
The error message hint now reads:
Unable to load translation and all the fallback languages, did you misspell the scope name?Does this PR introduce a breaking change?
Summary by cubic
Fixed the scope hint grammar in
TranslocoServicewhen a scoped language fails and all fallbacks fail. Added a test that simulates the scoped load + fallback failure and asserts "did you misspell the scope name?".Written for commit 06bc3c9. Summary will update on new commits.
Summary by CodeRabbit
Bug Fixes
Tests