search

longshotlabs / js-message-box

A package for defining and getting validation error messages, with support for Meteor Tracker reactivity
MIT License
7 stars 17 forks source link

Don't use handlebars #3

Closed clayne11 closed 7 years ago

clayne11 commented 7 years ago

handlebars is a huge dependency (73KB minified) and it's only used for essentially template strings. You could just as easily do a string replace given how simple the templates being used in this library are.

I think we should remove the handlebars dependency and change our formatting technique to something much more lightweight.

aldeed commented 7 years ago

Well actually it was a simple string replace in the previous version of this and I switched to handlebars because I didn't want to have to do custom support for everything everyone was asking for. :) Maybe there would be a way to specify which you want and make the dependency optional.

aldeed commented 7 years ago

I'm thinking probably it should take the handlebars object as an option, like with tracker.

clayne11 commented 7 years ago

Isn't that the point of allowing a function instead of a string for messages? If someone really wants to use Handlebars they can plug it into the function that returns the label.

IMO the library should have a simple solution - just a string replace - and users that want more advanced handling of labels should use the function options and return whatever they see fit.

Handlebars is a large library, it doesn't make sense to bloat the library to appease a small percentage of users.

JulianKingman commented 7 years ago

This makes this package incompatible with react native, as handlebars requires fs, which doesn't exist in RN. By extension, node-simple-schema is also incompatible with RN (could be solved by this: https://github.com/aldeed/node-simple-schema/issues/53)

macrozone commented 7 years ago

I tried to get rid of messagebox completly on a fork of simpl-schema, but its not so easy, because the simpleSchema.messageBox property is also used in others of aldeed's packages.

I think the best would be to go a step back and replace handlebars here with a more simple string replace and release a major version that is incompatible in terms of string replacing.

DavidSichau commented 7 years ago

I just analysed my app in the most rc of meteor 1.5 and handlebars is about 20% of the overall code size of my whole app.

Would it be possible to add two alternatives. One which supports handlebars and one where es6 string literals are used?

aldeed commented 7 years ago

Merged PR and published in 0.1.0. Thanks all!