bruth / jsonpatch-js

A JavaScript implementation of the JSON Media Type for partial modifications: http://tools.ietf.org/html/rfc6902
http://bruth.github.io/jsonpatch-js
BSD 2-Clause "Simplified" License
181 stars 24 forks source link

Compile as a Common JS module #24

Closed pallix closed 7 years ago

pallix commented 8 years ago

Hi,

my team and I would be interested to use this library for a CouchDB application prototype. CouchDb requires external libraries to support the Common JS format. Would it be possible to support it?

Thank you very much.

bruth commented 8 years ago

The module uses the universal module definition wrapper which does (should) support CommonJS. This is the header which checks for the exports variable. Is there another structure or format that is more conventional?

pallix commented 8 years ago

I'm not sure. I'm using this other library without problem in Couch but would need more time to invest what is the difference in the header with your library :/

bruth commented 8 years ago

😉 I agree that is a mess to read. A better question may be, did you try using this library, importing it as a Common JS module? I am assuming you are asking before it is failing? I don't development in a Node environment myself, but I was able to import it using the REPL.

screen shot 2016-07-28 at 7 13 01 pm
pallix commented 8 years ago

Yes it works in Node but not in the CouchDB environment. There is a small difference between Node and Common JS modules and CouchDB does not use the Node variant.

There are some guidelines on UMD here: https://github.com/umdjs/umd

And also in this example: https://github.com/umdjs/umd/blob/master/templates/commonjsStrict.js#L25

the first argument to factory is exports whereas in your code:

https://github.com/bruth/jsonpatch-js/blob/master/jsonpatch.coffee#L8

it is root, also the condition checked in both codes are really different.

What do you think?

pallix commented 8 years ago

My intuition was right, I made this ugly hack in the source code:

        (function(root, factory) {
    if (typeof define === 'function' && define.amd) {
            // AMD. Register as an anonymous module.
            define(['exports'], factory);
        } else if (typeof exports === 'object' && typeof exports.nodeName !== 'string') {
            // CommonJS
            factory(exports);
        } else {
            // Browser globals
            factory((root.jsonpatch = {}));
        }
    // if (typeof exports !== 'undefined') {
    //   return factory(root, exports);
    // } else if (typeof define === 'function' && define.amd) {
    //   return define(['exports'], function(exports) {
    //     return root.jsonpatch = factory(root, exports);
    //   });
    // } else {
    //   return root.jsonpatch = factory(root, {});
    // }

and now it works in Couch!

So there is something not right with the way modules are defined. I'm not sure sure what the proper fix it yet but reading https://github.com/umdjs/umd should help. However I'm short on time to submit a patch and also I have not a lot of experience with JS.

bruth commented 7 years ago

Feel free to reopen if there is still an issue.