browserify attempts to load all language files

[clayzermk1]

The patch in #66 automatically loads all language files for any commonjs scenario, browserify included.

While I can't really complain about doing this on the server with Node.js, doing this in the browser adds unnecessary bloat that I can't easily remove.

Possible solutions off the top of my head:

• provide a way to specify an array of languages which defaults to all languages on Node
• create special logic for Node within the commonjs scope

As a side effect, this might help with #70.

[clayzermk1]

Reverting #66 fixes this issue along with #70.

[BenjaminVanRyseghem]

But it also reverse the fix for #62 😕

[clayzermk1]

Perhaps there is a way of implementing #62 without putting it in the module definition? Can we make a loader function that then uses the appropriate loader for the framework?

[clayzermk1]

Submitted a PR for a trivial patch to check process.title === 'node'.

I know browserify uses "browser" and I don't know for certain about WebPack but I don't think it uses "node".

I don't think this is an ideal solution to the overall problem - it just punts it to another day but gets browserify folks back up and running without breaking #62.

[clayzermk1]

@BenjaminVanRyseghem have you had the chance to give this topic any further thought? I'd like to get a solution going that goes in the right direction for everyone and the project in general.

[BenjaminVanRyseghem]

the process.title === 'node' solution is not the most elegant one, but it seems to work.

It's better to have a half ugly working solution than a theoretical perfect not working solution 😄

Then maybe the browserify users have more input on this?

[clayzermk1]

Agreed, like I said, it's a punt.

The issue with the module definition is that Node.js specific code got placed in a CommonJS section. This would theoretically break all CommonJS module loaders that aren't Node.js. Browserify (#70) and WebPack (#74) (CommonJS implementers) are clearly broken because of this.

I feel like I'm taking this issue off topic though. My point of this issue was to address all languages modules being loaded when using a CommonJS implementation. That should not be done on the client, and in my opinion it should not be done in the general case. I think a language loading function is a superior solution. It's hardly troublesome to have users specify which locales they would like to load in addition to the default en_XX locale.

I envision something like the following:

var numbro = require('numbro').includeLocales([ 'jp_JP', 'de_DE' ]); // loads just Japanese and German locales
var numbro = require('numbro').includeAllLocales(); // loads all locales, can be an alias to includeLocales()

[BenjaminVanRyseghem]

yep, it looks like this whole part was a hack to have it out, but it feels like it's time to have a look at it now 😄

your API proposal sounds good to me 😄 , if you want to propose a PR going in this direction, that will be awesome 😄

[clayzermk1]

I'll see what I can do, I have a feeling it'll take some time to fix all the tests etc.

Is anyone else working towards a solution for this?

[BenjaminVanRyseghem]

I can help if you want, but I have really little time available those days

(I created a Gitter room for numbro if you want to discuss there)

[clayzermk1]

#71 seems to related as well.

[BenjaminVanRyseghem]

can you check that #101 fixes this issue?

[clayzermk1]

I can confirm that this issue is resolved by #101.