Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

@rmja
Copy link
Contributor

rmja commented Nov 9, 2019

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

@zewa666

This comment has been minimized.

Copy link
Member

zewa666 commented Nov 9, 2019

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

This comment has been minimized.

Copy link

Sayan751 commented Nov 9, 2019

@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

This comment has been minimized.

Copy link
Contributor Author

rmja commented Nov 9, 2019

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.

@rmja rmja closed this Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.