Skip to content

Conversation

@ReneWerner87
Copy link
Member

@ReneWerner87 ReneWerner87 commented Oct 31, 2025

fixes #1142


https://chatgpt.com/codex/tasks/task_e_69038608cf1c8326b3922f65fa0ff45d

Summary by CodeRabbit

  • Breaking Changes

    • Middleware-style API replaced by a reusable translator container — initialization and localization calls must migrate to the new instance-based model.
  • New Features

    • Translator container supports configurable default language, custom loaders and language handlers.
  • Improvements

    • Safer default handling to avoid shared mutable state and clearer error messages; improved concurrent localization behavior.
  • Tests / Documentation

    • Tests updated for concurrency and resource management; README includes migration guidance.

@ReneWerner87 ReneWerner87 requested a review from a team as a code owner October 31, 2025 12:16
@ReneWerner87 ReneWerner87 requested review from gaby and sixcolors and removed request for a team October 31, 2025 12:16
@ReneWerner87 ReneWerner87 requested a review from efectn October 31, 2025 12:16
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

The i18n package is refactored from a per-request middleware to a reusable, thread-safe container. New() now returns *I18n; Localize/MustLocalize are instance methods. Config removes Next and internal mutex; tests and examples updated to use a shared translator instance.

Changes

Cohort / File(s) Summary
Documentation & Examples
v3/i18n/README.md, v3/i18n/example/main.go
README and example updated to document container API; example switched from app.Use(New(...)) to translator := i18n.New(...) and calls changed to translator.Localize / translator.MustLocalize; migration guidance added.
Core Implementation
v3/i18n/i18n.go
Introduced I18n container type; New(...)*I18n constructor and prepareConfig() helper added; Localize/MustLocalize converted to methods on *I18n; removed middleware return and package-level locals key; improved nil/error handling.
Configuration
v3/i18n/config.go
Config struct changed: removed Next and mu; defensive copying of defaults implemented; defaultLangHandler and config defaulting logic simplified and safer against shared mutable state; UnmarshalFunc, Loader, DefaultLanguage, LangHandler wiring updated.
Tests
v3/i18n/embed_test.go, v3/i18n/i18n_test.go
Tests updated to create and pass a shared *I18n translator instead of using middleware; package-level calls replaced with instance methods; added TestTranslatorConcurrentLocalize; ensured response bodies are closed in tests.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant I18n as i18n Container
    participant Localizer as go-i18n

    App->>I18n: New(config) → *I18n
    activate I18n
    I18n->>I18n: prepareConfig()
    I18n->>Localizer: build bundle & localizers
    deactivate I18n

    App->>App: register route handlers

    App->>I18n: translator.Localize(ctx, params)
    activate I18n
    I18n->>I18n: resolve language (ctx -> LangHandler)
    I18n->>Localizer: get localizer & localize(config)
    Localizer-->>I18n: translated string / error
    I18n-->>App: string or error
    deactivate I18n
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

  • Areas requiring extra attention:
    • prepareConfig() and defensive copying of AcceptLanguages in config.go
    • Removal of mutexes and verifying concurrency safety in i18n.go (localizer map access)
    • Lifecycle and initialization changes in New() and bundle/localizer setup
    • Tests that exercise concurrent use (TestTranslatorConcurrentLocalize) and any added goroutine paths
    • API compatibility and migration notes in README.md

Possibly related PRs

Suggested labels

v3, ❗ BreakingChange

Suggested reviewers

  • gaby
  • sixcolors
  • efectn

Poem

🐰 I stitched a translator, neat and small,
No middleware hopping from call to call,
Shared and steady, beneath the moonlight,
I whisper translations—soft and right. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The pull request title "Remove middleware Next hook from i18n config" accurately describes one aspect of the changes—the Config.Next field is indeed removed from the i18n configuration. However, the changeset encompasses a much larger architectural refactoring that transforms the entire i18n package from a middleware-based pattern to a reusable container-based pattern. The title refers to a real part of the change but does not capture the main point of the refactoring, making it partially related to the overall changeset rather than a comprehensive summary of the primary changes.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4403a3a and 99eae2c.

📒 Files selected for processing (6)
  • v3/i18n/README.md (3 hunks)
  • v3/i18n/config.go (2 hunks)
  • v3/i18n/embed_test.go (2 hunks)
  • v3/i18n/example/main.go (1 hunks)
  • v3/i18n/i18n.go (4 hunks)
  • v3/i18n/i18n_test.go (9 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @ReneWerner87, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the i18n package by transforming it from a Fiber middleware into a dedicated, thread-safe I18n object. This change aims to provide a more flexible and efficient localization solution by allowing the I18n instance to be initialized once and reused across application handlers, thereby removing the need for middleware-specific configuration like the Next hook and simplifying the overall setup.

Highlights

  • Refactored i18n package: The i18n package has been refactored from a Fiber middleware into a standalone, reusable, and thread-safe I18n instance. This means localization is no longer handled via app.Use() middleware.
  • Removed Next hook: The Next hook, previously used for skipping middleware, has been removed from the i18n.Config struct as it's no longer relevant in the new architecture.
  • Updated API for localization functions: The Localize and MustLocalize functions are now methods of the *I18n instance, promoting better encapsulation and explicit usage.
  • Simplified configuration initialization: The default configuration initialization logic has been simplified and improved to prevent shared mutable state, especially for AcceptLanguages.
  • Enhanced test suite: Tests have been updated to reflect the new API, include t.Parallel() for concurrent execution, and ensure proper resource cleanup with resp.Body.Close(). A new test TestTranslatorConcurrentLocalize was added to verify thread-safety.
  • Migration guide added: The README.md now includes a detailed migration guide for users transitioning from the old middleware-based approach to the new I18n instance.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request is a significant improvement to the i18n package. It refactors the package from a middleware-based approach to a more flexible and efficient thread-safe container. The removal of the Next hook and the introduction of the I18n struct are well-executed. The changes make the package's API clearer and its usage more intuitive, as demonstrated by the updated examples and the new migration guide in the README.

Key improvements include:

  • A cleaner, non-middleware API that is easier to use and test.
  • Enhanced robustness by creating copies of default configurations, preventing unintended side effects on the global default settings.
  • Improved test suite, including fixes for resource leaks (Body.Close()) and the addition of a crucial concurrency test to validate the thread-safety of the new design.
  • Code cleanup, such as removing redundant checks.

The changes are thorough and of high quality. I have no specific comments for code changes. Great work!

@ReneWerner87 ReneWerner87 changed the title Remove middleware Next hook from i18n config i18n: Change middleware usage to global container Oct 31, 2025
@ReneWerner87
Copy link
Member Author

@coderabbitai pls update the summary

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 31, 2025

@ReneWerner87: I'll regenerate the summary for this PR.


🧠 Learnings used
Learnt from: ReneWerner87
Repo: gofiber/contrib PR: 0
File: :0-0
Timestamp: 2024-07-03T11:59:00.303Z
Learning: The i18n functionality in the gofiber/contrib repository is being refactored from middleware to a global container to improve robustness and performance. The global container will be initialized once before setting up routes and will manage the i18n bundle and localizer map.

Learnt from: ReneWerner87
Repo: gofiber/contrib PR: 0
File: :0-0
Timestamp: 2024-10-16T10:04:06.328Z
Learning: The i18n functionality in the gofiber/contrib repository is being refactored from middleware to a global container to improve robustness and performance. The global container will be initialized once before setting up routes and will manage the i18n bundle and localizer map.
✅ Actions performed

Summary regeneration triggered.

@ReneWerner87 ReneWerner87 merged commit 1bcd94f into main Oct 31, 2025
5 checks passed
@ReneWerner87 ReneWerner87 deleted the codex/2025-10-31-12-16-20 branch October 31, 2025 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🚀 [Feature]: i18n refactoring from middleware to global container

2 participants