tandrewnichols/indefinite

Plural should not have indefinite article

gajus opened this issue ยท 5 comments

gajus commented
// current
indefinite('French Fries') => 'a French Fries'

// expected
indefinite('French Fries') => 'French Fries'
gajus commented

Quick workaround:

// @flow

import indefinite from 'indefinite';
import pluralize from 'pluralize';

export default (subject: string): string => {
  if (pluralize(subject) === subject) {
    return subject;
  }

  return indefinite(subject);
};

Currently indefinite doesn't attempt to know whether something should have an indefinite article or not, only which one to use (a vs an). To this point, I've relied on consumers to know whether it should or shouldn't have one (and call indefinite accordingly). For that reason, I'm curious to hear more about your use case. Is there a reason you can't just not call it? I could probably support it using your workaround, but I don't know if I should . . . I don't know if it's within the scope of the project because

  1. I have to pull in pluralize, which doesn't matter for the server-side, but adds a lot of weight to the client-side that everyone has to download but most won't use.
  2. I'm then relying on pluralize not to get it wrong. Like, can I know with 100% certainty that pluralize(subject) === subject will always work? What about for irregulars?
  3. Maybe for separation of concerns it's better to let consumers handle it (as you've done).

To that last point, it's obviously sort of related, so I can see a case for including it, but the other two points seem prohibitive at the moment. If I added this, I think I'd maybe not want to use pluralize, but then I'm not sure I want to try to maintain whatever I wrote in its place.

gajus commented

Just document the workaround in the docs. No need to depend on pluralize for the reasons you've suggested.

Sounds good. Thanks.

Updated the readme.