`BaseModel.toObject()` fails with `null` nested BelongsTo relationship

[theRealPadster]

Package version

18.4.2

Describe the bug

I have a model that has a nested nullable BelongsTo relationship, and when I try to use .toObject() on it, it gives me a TypeError: Cannot read properties of null (reading 'toObject')

The model is like this: Part > Item > PartCategory PartCategory has a parent_id: number | null column field, and a parent relationship like so:

@column()
public parent_id: number | null;

@belongsTo(() => PartCategory, {
  foreignKey: 'parent_id',
})
public parent: BelongsTo<typeof PartCategory>;

When I query my Part model, it works if the category has a parent, but fails when the category has no parent.

const partAd = await Part.query()
  .where('slug', params.slug)
  .andWhere('status', 'ACTIVE')
  .preload('item',
    (q) => q.preload('category',
      (q2) => q2.preload('parent')
    )
  )
.firstOrFail();

const partAdObj = partAd.toObject();

I was able to fix this by overriding BaseModel.toObject() in my PartCategory model, to check for falsey values before trying to call .toObject() on them.

  /**
   * Convert model to a plain Javascript object
   */
  public toObject() {
    const Model = this.constructor;
    const computed = {};
    /**
     * Relationships toObject
     */
    const preloaded = Object.keys(this.$preloaded).reduce((result, key) => {
      const value = this.$preloaded[key];
      if (!value) {
        return result;
      }
      result[key] = Array.isArray(value) ? value.map((one) => one.toObject()) : value.toObject();
      return result;
    }, {});
    /**
     * Update computed object with computed definitions
     */
    // @ts-ignore
    Model.$computedDefinitions.forEach((_, key) => {
      const computedValue = this[key];
      if (computedValue !== undefined) {
        computed[key] = computedValue;
      }
    });
    return {
      ...this.$attributes,
      ...preloaded,
      ...computed,
      $extras: this.$extras,
    };
  }

Is what I'm doing correct? Is this a bug, or am I doing something wrong? I can submit a PR with this fix if I'm correct in my diagnosis.

Full stack trace:

TypeError: Cannot read properties of null (reading 'toObject')
    at /app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1507:93
    at Array.reduce (<anonymous>)
    at Proxy.toObject (/app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1505:56)
    at /app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1507:93
    at Array.reduce (<anonymous>)
    at Proxy.toObject (/app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1505:56)
    at /app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1507:93
    at Array.reduce (<anonymous>)
    at Proxy.toObject (/app/node_modules/@adonisjs/lucid/build/src/Orm/BaseModel/index.js:1505:56)
    at new AdObject (/app/app/Libraries/Algolia/classes/AdObject.ts:16:25)

Reproduction repo

No response

[thetutlage]

Before you create a PR, can you please create a failing test in this repo? Lucid has many moving parts, so first we should be able to reproduce the issue in a test before we can accept any PRs for the code change

[theRealPadster]

Sure, I can try. Do you mean submit a PR with the test? I'll admit I've not done any tests with Adonis/Japa before, so might need to figure some stuff out first. Would this be the correct place to add it? And since my project is still on Adonis v5, it's using Lucid 18. Is there a branch or something I should use as a base?

[thetutlage]

Yes, it should be based on https://github.com/adonisjs/lucid/tree/v18 branch.

• I think, you just have to clone the repo.
• Install dependencies
• Run tests using npm run test:sqlite. This way you will not need any full blown SQL servers running on your computer.
• To run your specific tests, you can pin it using the .pin method. Learn more in Japa docs.

Feel free to ask for help if you feel stuck

[theRealPadster]

Okay, I've got this so far:

// TODO: I'm not really checking the author, so can probably just remove...
  test('add preloaded belongsTo relationship to toObject result', async ({ assert }) => {
    class Category extends BaseModel {
      @column()
      public name: string

      @column()
      public parentId: number

      @belongsTo(() => Category)
      public parent: BelongsTo<typeof Category>
    }

    class Post extends BaseModel {
      @column()
      public title: string

      @column()
      public userId: number

      @belongsTo(() => User)
      public author: BelongsTo<typeof User>

      @hasOne(() => Category)
      public category: HasOne<typeof Category>
    }

    class User extends BaseModel {
      @column({ isPrimary: true })
      public id: number

      @column()
      public username: string

      @hasMany(() => Post)
      public posts: HasMany<typeof Post>
    }

    const user = new User()
    user.username = 'virk'

    const category = new Category()
    category.name = 'Tutorials'

    const subCategory = new Category()
    subCategory.name = 'Lucid'

    const post = new Post()
    post.title = 'Adonis 101'

    // user.$setRelated('posts', [post])
    post.$setRelated('author', user)
    subCategory.$setRelated('parent', category)
    post.$setRelated('category', subCategory)

    // passes
    assert.deepEqual(subCategory.toObject(), {
      name: 'Lucid',
      parent: {
        name: 'Tutorials',
        $extras: {},
      },
      $extras: {},
    })

    // fails
    assert.deepEqual(category.toObject(), {
      name: 'Tutorials',
      parent: null,
      $extras: {},
    })
  }).pin()

I don't think I actually need the Post/Author stuff, so should I remove that? I'm also not sure about the parent being null, since we're not actually setting the parent_id in this code, just using $setRelated, which I haven't used before.

I can just make a draft PR with what I have if that's better than here :)