jeremyfa / yaml.js

Standalone JavaScript YAML 1.2 Parser & Encoder. Works under node.js and all major browsers. Also brings command line YAML/JSON conversion tools.
MIT License
890 stars 142 forks source link

Loader used is convoluted and inconvenient #52

Open minj opened 8 years ago

minj commented 8 years ago

Since I don't work with node or coffeescript, I do not know much about all the loaders out there but the one you use here is certainly of doubtful quality when it comes to non-node contexts where require is not available...

One such context I deal with is Firefox addons. It's useful to isolate script scopes completely when including them in Firefox to prevent global scope pollution. What is more, window object may not even be available here, and even if it is, it's probably a XUL window.

Long story short, { module: { exports: {} } } is the minimal global object but your loader does not handle it at all.

Even if you do set YAML on this and module.exports on lines 1859+ they are not actually exported anywhere: both this and module.exports are references to l.exports from var l=n[o].

Sure n[o].exports is a return value for s but it's not actually used:

for (var o = 0; o < r.length; o++)
    s(r[o]);
return s

Having read all this minimized gibberish 'I can't even` is the only thing that comes to my mind.

The loader in js-yaml.js works just fine but there are other problems with that lib that prevents me from using it in an add-on.

jeremyfa commented 8 years ago

I don't know much about firefox addons, but if you need standard commonjs modules, you might rather use https://github.com/jeremyfa/yaml.js/blob/develop/lib/Yaml.js as entry point instead of the minified browserify version.

That said it's only tested with node.js environment (and the browserify version for regular browser environment), so you might have other issues (especially regarding the YAML.load method).

The this.YAML = ... is indeed completely useless on the browserify version. Not even sure we should keep it on the commonjs/node.js version.