Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces translation support using a filesystem loader and updates related services to handle translation files more flexibly. Key changes include:
- Introducing a new FSLoader implementation in translation/fs_loader.go to load translation JSON files using fs.FS.
- Adding comprehensive tests for FSLoader functionality in translation/fs_loader_test.go.
- Updating the service provider in translation/service_provider.go to detect and use FSLoader when provided with an fs.FS, and adjusting the file loader return type in translation/file_loader.go.
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| translation/fs_loader.go | Implements FSLoader for translations using fs.FS and JSON unmarshaling. |
| translation/fs_loader_test.go | Adds unit tests and benchmarks to validate FSLoader behavior. |
| translation/service_provider.go | Updates registration to use FSLoader when the language path is of type fs.FS. |
| translation/file_loader.go | Adjusts NewFileLoader signature to conform to the Loader interface. |
Files not reviewed (1)
- tests/go.mod: Language not supported
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #974 +/- ##
==========================================
+ Coverage 70.04% 70.09% +0.04%
==========================================
Files 168 169 +1
Lines 11353 11381 +28
==========================================
+ Hits 7952 7977 +25
- Misses 3050 3053 +3
Partials 351 351 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hwbrzzl
left a comment
There was a problem hiding this comment.
Nice 👍 Could you enrich the description to show how to use the new feature?
and pass |
|
Where should |
Anywhere, if use capital letters. |
|
I mean should a new go file be created like the image below? 
The process is similar with the comment, we just implement inject the FS to the lang configuration. |
|
Can FSLoader and FileLoader be used together instead of being mutually exclusive? I am thinking of having FileLoader override FSLoader. This way, when the embedded language package project goes live, we can use FileLoader to replace or add new language entries, making it more flexible. |
Yes |
Will optimize it. |
@almas1992 Do you mean that user doesn't need to rebuild project to add the lang file via the FileLoader? And the FSLoader is used at that time. |
Initially, FSLoader is used to load the embedded language files. Users can use FileLoader to load new language files without needing to rebuild. |
# Conflicts: # tests/go.sum
| func (t *TranslatorTestSuite) TestLoadFS() { | ||
| mockFS := fstest.MapFS{ | ||
| "lang/cn/test.json": &fstest.MapFile{Data: []byte(`{"foo": "bar", "baz": {"foo": "bar"}}`)}, | ||
| } | ||
|
|
||
| translator := NewTranslator(t.ctx, NewFSLoader("lang", mockFS, json.NewJson()), t.mockLoader, "en", "en", t.mockLog) | ||
| t.mockLoader.On("Load", "en", "test").Once().Return(map[string]any{ | ||
| "foo": "one", | ||
| "bar": "two", | ||
| }, nil) | ||
|
|
||
| // Case: Not loaded, successful load | ||
| err := translator.load("en", "test") | ||
| t.NoError(err) | ||
| t.Equal("one", loaded["en"]["test"]["foo"]) | ||
|
|
||
| // Case: Already loaded | ||
| err = translator.load("en", "test") | ||
| t.NoError(err) | ||
| t.Equal("two", loaded["en"]["test"]["bar"]) | ||
|
|
||
| // Case: Loaded from FS | ||
| t.mockLoader.On("Load", "cn", "test").Once().Return(nil, errors.LangFileNotExist) | ||
| err = translator.load("cn", "test") | ||
| t.NoError(err) | ||
| t.Equal("bar", loaded["cn"]["test"]["foo"]) | ||
|
|
||
| // Case: Not loaded, loader returns an error | ||
| t.mockLoader.On("Load", "es", "folder3").Once().Return(nil, errors.LangFileNotExist) | ||
| err = translator.load("es", "folder3") | ||
| t.EqualError(err, "translation file does not exist") | ||
| t.Nil(loaded["folder3"]) | ||
| } | ||
|
|
📑 Description
Closes goravel/goravel#456
✅ Checks