rxaviers / react-globalize-compiler

I18n support for React applications using Globalize
MIT License
5 stars 2 forks source link

Make requirejs a dependency #7

Closed necolas closed 8 years ago

necolas commented 8 years ago

Not sure why requirejs is a peerDependency, but npm@3 complains about it being missing and the app needing to install it.

rxaviers commented 8 years ago

requirejs isn't a peer dependency, it is a residue from previous code. Fixed. Thanks

rxaviers commented 8 years ago

Published 0.0.7 with this fix.

necolas commented 8 years ago

There's a require("requirejs") in this file though: https://github.com/rxaviers/react-globalize-compiler/blob/master/lib/dynamic-compiler.js

rxaviers commented 8 years ago

Thanks for pointing it out. The dynamic compiler was the previous code I had in mind, but I forgot it was still linked by index.js. Having said that, I regret making this incomplete-change a patch fix. My second attempt to fix this issue then is: (a) revert my attempt-one in a new patch release; (b) properly fix this in a new minor release.

Please, just let me know if you still find any problem.

necolas commented 8 years ago

That resolves the npm3 issue, thanks. However, the dynamic compiler won't work without requirejs being installed.

rxaviers commented 8 years ago

You're welcome.

However, the dynamic compiler won't work without requirejs being installed.

I assume you aren't using it, but you're concerned about the correctness of the "solution". Please, correct me if I'm wrong and you're using the dynamic one.

I'm favoring the static compiler over the other here, because it's the one being used both in Globalize examples and react-globalize. You're correct that the dynamic compiler won't work without requirejs being installed. Peer dependency was correctly defined for that, the problem is that we cannot have optional peer dependencies. Therefore, a saner approach for the dynamic one would be to move it to a different project.

rxaviers commented 8 years ago

I'm also open for ideas.