lb-ng Generates duplicate keys in object JSON when remote method is defined both in js and json files

[cmoore4]

If a remote method is defined in a model's JS file and in its JSON file, if you run lb-ng on the models, the angular sdk outputs an object with duplicate keys(one key from the JSON definition, one from the js implementation). This violates the 'use strict' declaration of the function, and causes the file not to load at all.

  "methods": {
    "greet": {
      "isStatic": true,
      "accepts": {
        "arg": "msg",
        "type": "string"
      },
      "returns": {
        "arg": "greeting",
        "type": "string"
      },
      "description": "Provided a name, returns a greeting ",
      "http": [
        {
          "httpPath": "greet",
          "httpVerb": "post"
        }
      ]
    }
  }

  ModelWithRemoteMethod.greet = function(msg, cb) {
    cb(null, 'Greetings... ' + msg);
  };

  ModelWithRemoteMethod.remoteMethod( 
    'greet',
    {
      http: {path: '/greet', verb: 'post'},
      accepts: {arg: 'msg', type: 'string'},
      returns: {arg: 'greeting', type: 'string'}
    }
  );

Generates this:
https://github.com/cmoore4/loopback-angular-remote-method-duplicate-key-bug-demo/blob/master/client/lb-services.js#L1606-L1643

        "greet": {
          url: urlBase + "/mwrms/greet",
          method: "POST"
        },

        /**
         * @ngdoc method
         */
        "greet": {
          url: urlBase + "/mwrms/greet",
          method: "POST"
        },

Removing the method from the JSON definition resolves the issue, but is not optimal.

See demo project here:
https://github.com/cmoore4/loopback-angular-remote-method-duplicate-key-bug-demo

[0candy]

Is there a reason why you added the remote method to both the model js file and the json file?

[cmoore4]

Hello Candy,

There's very little documentation around the "methods" section in the model json file,

https://docs.strongloop.com/display/public/LB/Model+definition+JSON+file#ModeldefinitionJSONfile-Methods

You can declare remote methods here. Until this feature is implemented, you must declare remote methods in code.

I assumed this meant I could declare them in the json file, but also had to declare them in code. Pragmatically, I like my whole API contract to be easily groked by my developers within the model's json file and not to have mystery endpoints hidden in the implementation code.

I see now that what the documentation really means is that I should not declare anything in methods within the json. But the code in my pull request #214 will prevent hidden gotchas from future developers.

In our case, the issue wasn't surfaced until a client demo, as "use strict" is not implemented the same across all browsers and devices. The generated code happily ran in latest Chrome and FF on Win7, and on my Android 4.4 device. It failed on Android 5.1, though. It ran on Chrome on iPhone 5S, but not on Safari on iPhone 5S, and not at all on iPhone 6 and up. Was easy enough to debug once we got the device back into our lab, but we never caught it while smoke screening during development.

I'm not saying this is necessarily a bug in Loopback, but this bit of defensive code could prevent issues moving forward. The other option, I suppose, is to throw an error, probably best done while loading the Loopback models in the first place, not in loopback-sdk-angular. But this is the solution I rolled out to our package.json to solve our immediate needs.

[0candy]

Hi Sean,
Thank you for the clarification. I can see that the documentation can cause confusion. Once you get a chance to add unit tests to your PR, we can get that reviewed and merged in.

[davidcheung]

perhaps the solution to this is when declaring remoteMethod for a model we should see if theres duplicating path. then throw a warning from model itself, since this would be a problem with all the SDKs too? https://github.com/strongloop/loopback/blob/f9a58083ae247590c6f9541104200f6bc8d1b4cb/lib/model.js#L421

[davidcheung]

Closing this issue now, if we want preventive checks and warn, i believe it shouldn't happen in the SDK level.