QuiiBz / next-international

Type-safe internationalization (i18n) for Next.js
https://next-international.vercel.app
MIT License
1.25k stars 59 forks source link

getI18n(scope) === getScopedI18n(scope) #230

Closed rentalhost closed 11 months ago

rentalhost commented 11 months ago

Is your feature request related to a problem? Please describe. I would like to suggest a modification to the getI18n() and useI18n() functions in order to improve their usability and reduce redundancy.

Describe the solution you'd like I propose updating the signature of the getI18n() and useI18n() functions to accept an optional scope parameter, like this: getI18n(scope?: string) and useI18n(scope?: string). This change would allow the functions to handle both scoped and non-scoped usage, making the getScopedI18n() and useScopedI18n() functions redundant.

Describe alternatives you've considered An alternative approach would be to keep the existing functions as they are, maintaining separate getI18n() and useI18n() functions alongside the scoped versions. However, this would result in duplicated functionality and could be less intuitive for users who need both scoped and non-scoped access to internationalization features.

Additional context This change would provide a more streamlined and user-friendly API for working with internationalization in the next-international project. It simplifies the usage of these functions by allowing users to specify the scope when needed, while still supporting the existing functionality. This modification aligns with the goal of reducing unnecessary complexity and redundancy in the codebase.

QuiiBz commented 11 months ago

See https://github.com/QuiiBz/next-international/issues/27 (specifically https://github.com/QuiiBz/next-international/issues/27#issuecomment-1220282456) where we discussed this exact pattern, but landed on having two separate functions.

I don't think going backward to useI18n(scope) is a good idea, as it changes the signature of the hook and can be more confusing than an explicit useScopedI18n(scope) hook.

We also don't want to introduce heavy breaking changes considering the library is now in stable v1. We could deprecate useScoped18n(scope) but that means maintaining more hooks and adding again more confusion.

gustaveWPM commented 11 months ago

On the contrary, I don't think it's a good thing to want to mix everything in the same function.

For me, the case where it's really relevant to use scopedI18n is when no getI18n call has been made previously in the component, because I don't like having to induce unnecessary awaits or intermediate calculations. In the end, its usecase is fairly exceptional imo.

rentalhost commented 11 months ago

I understand that a layout decision was made considering two functions. In the end, indeed, we would have two functions doing the same thing in a way, and a possible deprecation of the scoped version since it would become redundant. Although I personally prefer a single function (as other libraries do, like next-intl), perhaps there is no longer a need to discuss this further, precisely because of the stable v1.

Lastly, I just thought it would be better because the implementation of both methods is perfectly identical, using undefined with scope to getI18n() internally (1, 2).

Furthermore, I believe that on my side, as an exception, I can implement a method that does this automatically for me in client.ts and server, for example.

Thank you, anyway!

QuiiBz commented 11 months ago

Yeah you should be able to achieve this on your side if you really want too, especially since the next release will expose the internal createT function that is used in both useI18n() / useScopedI18n: https://github.com/QuiiBz/next-international/issues/229