search

ulion / jsonform

Build forms from JSON Schema. Easily template-able. Compatible with Twitter Bootstrap out of the box.
http://ulion.github.io/jsonform/playground/
MIT License
49 stars 27 forks source link

Fix for Browserify / similar require #30

Closed fuhrysteve closed 8 years ago

fuhrysteve commented 8 years ago

Hi @ulion! Looking at this I don't know if it has been working properly since 2012. Anyways, this seems to fix it for me and the module works with browserify now (if you leak jquery to the global scope like so):

since underscore is explicitly defined as null if it is undefined, we should check for whether it is null rather than if it is undefined again.

For context, please see: https://github.com/ulion/jsonform/blob/e590a02ce6729458fdb564da065e2095c923531e/lib/jsonform.js#L4472

Thanks!

ulion commented 8 years ago

That 2 places, just check "!_" would be better imo.

fuhrysteve commented 8 years ago

To clarify, do you mean change this

if (serverside && _ === null) {

to this?

if (serverside && !_) {
ulion commented 8 years ago

and also the 3rd line since this line.

2016-03-27 19:49 GMT+08:00 Stephen J. Fuhry notifications@github.com:

To clarify, do you mean change this

if (serverside && _ === null) {

to this?

if (serverside && !_) {

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/ulion/jsonform/pull/30#issuecomment-202047552

Ulion

fuhrysteve commented 8 years ago

Ok, I made the truthiness change.

Looking closer at that third line - I don't think it's possible for it to ever get run unless _ is a module, and that module is also undefined - which might not even be possible.

Additionally, it appears that if require('_') fails, that it raises an exception - so that line shouldn't get executed either if underscore is not present.

Further - even if that line did get run, it doesn't look like it will work as designed since the line is missing typeof _

Thanks again, and what do you think, @ulion?