aurelia / i18n

A plugin that provides i18n support.
MIT License
93 stars 70 forks source link

feat(value-converter) Assume the argument is count if it is a number #311

Closed rmja closed 4 years ago

rmja commented 4 years ago

This makes it super easy to use the plural functionality in 18next, simply by ${"some-key" | t:20}.

zewa666 commented 4 years ago

well the same could be achieved by passing in an object with a count property. The issue with your approach would be that for every other combination we'd then have to support parameter overloads as well. Honestly I'm not sure about this, what do you say @Sayan751?

Sayan751 commented 4 years ago

@zewa666 I am with you on this. As i18next supports a wide range of translation use-cases, it is difficult to support every one of those and keep the API user-friendly at the same time. At this point, the API is such that it is aligned with i18next. That's how we can make sure that whatever new plugin or translation use case come in future, aurelia-i18n will support those without making any change (unless the i18next API changes). I think such rules will not only raise the need of supporting every parameter combination, it will also make the design very much error-prone.

@rmja As you see that the value converter is very simple. I would suggest you to create another value converter that does the same, and use that instead in your project. It might look something like the one below (not tested).

import i18next from "i18next";

import { I18N } from "../i18n";
import { valueConverter } from "aurelia-framework";

@valueConverter("tCount")
export class TCountValueConverter {
  public static inject() { return [I18N]; }

  constructor(private service: I18N) { }

  public toView(value: any, count: number) {
    return this.service.tr(value, { count });
  }
}

This should suffice your need, unless you really want to pass sometimes number and sometimes object to the same instance of t VC usage. In that case, I would like to point out that from design perspective, I don't find that to be very sound. Especially, when you can trivially encapsulate the count as { count }.

rmja commented 4 years ago

Ok, this is fine. I have just seen myself using the ${"key" | t:{count:20}} at so many places throughout my app, that I found the shorthand to be cleaner. I will make the alternative converter as you suggest @Sayan751.

Thanks.