Cropster / ember-l10n

A GNU gettext based localization workflow for Ember
MIT License
15 stars 7 forks source link

Add override option to ignore extract error #85

Closed iamareebjamal closed 3 years ago

iamareebjamal commented 3 years ago

If we use a non-literal string or a variable in any of the functions or helpers like t(value) instead of t('Pending'), an error is thrown saying "You need to pass a string as argument to l10n methods"

https://github.com/Cropster/ember-l10n/blob/1a50396dd5fed5a5e282f93f4e9eff3b451fac38/lib/commands/utils/parse-js.js#L29-L31

This is the correct behaviour to ensure best practices but sometimes we need to pass non-literal values which we receive from server or are looping over in template. For example, there are 7 possible states of an order: initializing, pending, completed, placed, canceled, refunded, expired

We have translated all them but we can't use t(order.status) because it'll throw an error while extracting. Similarly, if I have to show a filter for all these statuses in the template, I'd loop for them like:

{for each Order.status as |status|}
    <span>{{t status}}</span>
{/for}

It'll work in runtime but throw an error while extracting. So, I'm thinking of 2 possible cases:

  1. Add an option to show recoverable errors like this to warnings instead of halting errors
  2. Add an override option to ignore certain dynamic calls to the functions, and ignore errors in those. For example, <span>{{t status ignoreExtract=true}}</span>. This way, the behaviour remains opt in and people can continue receiving helpful errors elsewhere and only opt-out of extraction if there is no other way
mydea commented 3 years ago

You can use l10n.tVar() or `{{t-var}} for this case. This is ignored for extraction, and does exactly what you want.

As a side note, I would generally recommend against using this, if possible. I think generally a case like you mentioned would be better solved with a custom helper, e.g.:

export default class OrderStatusHelper extends Helper {
  @service l10n;

  compute([orderStatus]) {
    const map = {
      'OPEN': this.l10n.t('Open'),
      'CLOSED': this.l10n.t('Closed')
    };

    return map[orderStatus];
  }
}
{{#each order.status as |status|}}
  <span>{{order-status status}}</span>
{{/each}}

This is a bit more verbose, but way more explicit, and ensures all available statuses are always extracted/available.

iamareebjamal commented 3 years ago

Thank you. I studied the documentation and I don't know how I missed tVar. Your helper suggestion works if the project had just one place where it showed dynamic values but much of the UI in the project is built dynamically depending on the return values from server so it is not scalable. To maintain the extractable versions, we enumerate them in model statuses = [t('accepted'), ...]

Thank you