fix(relations): parse relations array

[LookinGit]

Fixed error when relations it's array in loadMany method Lazy eager loading

[coveralls]

Coverage increased (+0.003%) to 96.308% when pulling 3cea702 on LookinGit:relations/parser into bb64d8c on adonisjs:develop.

[RomainLanz]

May you change Array to Object[]

[LookinGit]

May be Array to String[] not Object[]?

[RomainLanz]

I didn't read the code, so if you receive an array of string here yes, String[] will be better 😄

[RomainLanz]

May you change Array to Object[]

[coveralls]

Coverage increased (+0.003%) to 96.308% when pulling 717129d on LookinGit:relations/parser into bb64d8c on adonisjs:develop.

[thetutlage]

Can u guys share more info what exactly the change is. It will be nice to know.

1. The code can be used to reproduce the error
2. What exception is raised without this fix

[LookinGit]

Hi, @thetutlage
Example error:
file app/Controllers/Http/UserController:

...
async show ({ params }) {
  const user = await User.findOrFail(params.id)
  await user.loadMany(['profile', 'role'])
  return user
}

return error

{
      "error": {
        "message": "relation.split is not a function",
        "name": "TypeError",
        "status": 500,
        "frames": [{
          "file": "node_modules/@adonisjs/lucid/src/Lucid/Relations/Parser.js",
          "method": "RelationParser.parseRelation",
          "line": 70,
          "column": 35,
          "context": {
            "start": 65,
            "pre": "   * @param  {Function}    callback\n   *\n   * @return {Object}\n   */\n  parseRelation (relation, callback = null) {",
            "line": "    let [name, nested] = relation.split(/\\.(.+)/)",
            "post": "\n    /**\n     * Setup nested relation when it exists\n     */\n    nested = nested ? { [nested]: callback } : null"
          }
        }, {
          "file": "node_modules/@adonisjs/lucid/src/Lucid/Relations/Parser.js",
          "method": "_.transform",
          "line": 35,
          "column": 35,
          "context": {
            "start": 30,
            "pre": "   *\n   * @return {Object}\n   */\n  parseRelations (relations) {\n    return _.transform(relations, (result, callback, relation) => {",
            "line": "      const parsedRelation = this.parseRelation(relation, callback)",
            "post": "      const existingRelation = result[parsedRelation.name]\n\n      /**\n       * Update existing relation properties when it already\n       * exists."
          }
        }, {
          "file": "node_modules/lodash/lodash.js",
          "method": null,
          "line": 13784,
          "column": 16,
          "context": {
            "start": 13779,
            "pre": "        else {\n          accumulator = {};\n        }\n      }\n      (isArrLike ? arrayEach : baseForOwn)(object, function(value, index, object) {",
            "line": "        return iteratee(accumulator, value, index, object);",
            "post": "      });\n      return accumulator;\n    }\n\n    /**"
          }
        }, {
          "file": "node_modules/lodash/lodash.js",
          "method": "arrayEach",
          "line": 537,
          "column": 11,
          "context": {
            "start": 532,
            "pre": "  function arrayEach(array, iteratee) {\n    var index = -1,\n        length = array == null ? 0 : array.length;\n\n    while (++index < length) {",
            "line": "      if (iteratee(array[index], index, array) === false) {",
            "post": "        break;\n      }\n    }\n    return array;\n  }"
          }
        }, {
          "file": "node_modules/lodash/lodash.js",
          "method": "Function.transform",
          "line": 13783,
          "column": 43,
          "context": {
            "start": 13778,
            "pre": "        }\n        else {\n          accumulator = {};\n        }\n      }",
            "line": "      (isArrLike ? arrayEach : baseForOwn)(object, function(value, index, object) {",
            "post": "        return iteratee(accumulator, value, index, object);\n      });\n      return accumulator;\n    }\n"
          }
        }, {
          "file": "node_modules/@adonisjs/lucid/src/Lucid/Relations/Parser.js",
          "method": "RelationParser.parseRelations",
          "line": 34,
          "column": 14,
          "context": {
            "start": 29,
            "pre": "   * @param  {Object}       relations\n   *\n   * @return {Object}\n   */\n  parseRelations (relations) {",
            "line": "    return _.transform(relations, (result, callback, relation) => {",
            "post": "      const parsedRelation = this.parseRelation(relation, callback)\n      const existingRelation = result[parsedRelation.name]\n\n      /**\n       * Update existing relation properties when it already"
          }
        }, {
          "file": "node_modules/@adonisjs/lucid/src/Lucid/EagerLoad/index.js",
          "method": "new EagerLoad",
          "line": 26,
          "column": 39,
          "context": {
            "start": 21,
            "pre": " * @class EagerLoad\n * @constructor\n */\nclass EagerLoad {\n  constructor (relations) {",
            "line": "    this._relations = RelationsParser.parseRelations(relations)",
            "post": "  }\n\n  /**\n   * Calls the eagerloading callback on the related instance\n   * query only when defined"
          }
        }, {
          "file": "node_modules/@adonisjs/lucid/src/Lucid/Model/index.js",
          "method": "Proxy.loadMany",
          "line": 1167,
          "column": 23,
          "context": {
            "start": 1162,
            "pre": "   * @param  {Object} eagerLoadMap\n   *\n   * @return {void}\n   */\n  async loadMany (eagerLoadMap) {",
            "line": "    const eagerLoad = new EagerLoad(eagerLoadMap)",
            "post": "    const result = await eagerLoad.loadForOne(this)\n    _.each(result, (values, name) => this.setRelated(name, values))\n  }\n\n  /**"
          }
        }, {
          "file": "app/Controllers/Http/UserController.js",
          "method": "UserController.show",
          "line": 34,
          "column": 17,
          "context": {
            "start": 29,
            "pre": "   * @param {Object} params\n   * @return {Promise.<*>}\n   */\n  async show ({ params }) {\n    const user = await User.find(params.id)",
            "line": "    await user.loadMany(['profile', 'role'])",
            "post": "    return user\n  }\n  async store () {}\n  async update () {}\n  async destroy () {}"
          }
        }]
      }
    }

this is due to the fact that the method _.transform passed different values for arrays and objects in method RelationParser.parseRelation():

• for array it's index
• for object it's value

[LookinGit]

Need add this fix or add the check on array in method RelationParser.parseRelations():

...
class RelationParser {
  parseRelations (relations) {
    const relationsIsArray = Array.isArray(relations)

    return _.transform(relations, (result, callback, relation) => {
      const parsedRelation = relationsIsArray
        ? this.parseRelation(callback)
        : this.parseRelation(relation, callback)

      const existingRelation = result[parsedRelation.name]
      ...
    }, {})
  }
}

[thetutlage]

Do we really need new Array here. Also it will be simpler to write _.transform here. Easy to understand

_normalizeRelations (relations) {
    if (Array.isArray(relations)) {
        return relations
    }

    return _.transform(relations, (result, relation ) => {
        return (result[relation] = null)
    }, {})
}

[coveralls]

Coverage increased (+0.06%) to 96.361% when pulling b085f8a on LookinGit:relations/parser into bb64d8c on adonisjs:develop.

[thetutlage]

Thanks 😄