Merge pull request #1839 from bookshelf/rg-empty-hasone

Empty hasOne relation should return null instead of {} when serialized
bookshelf · May 10, 2018 · 73b47dc · 73b47dc
2 parents 13ee6f5 + 21e4aa3
commit 73b47dc
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 14 deletions.
diff --git a/lib/eager.js b/lib/eager.js
@@ -72,8 +72,9 @@ class EagerRelation extends EagerBase {
 
   // Handles the eager load for both the `morphTo` and regular cases.
   _eagerLoadHelper(response, relationName, handled, options) {
-    const relatedModels = this.pushModels(relationName, handled, response);
-    const relatedData   = handled.relatedData;
+    const relatedData = handled.relatedData;
+    const isEmptyHasOne = response.length === 0 && relatedData.type === 'hasOne';
+    const relatedModels = isEmptyHasOne ? [] : this.pushModels(relationName, handled, response);
 
     return Promise.try(() => {
       // If there is a response, fetch additional nested eager relations, if any.

diff --git a/lib/model.js b/lib/model.js
@@ -106,6 +106,9 @@ const BookshelfModel = ModelBase.extend({
    *   than `Target` model's `id` / `{@link Model#idAttribute idAttribute}`.
    *
    * @returns {Model}
+   *
+   *   The return value will always be a model, even if the relation doesn't exist, but in that
+   *   case the relation will be `null` when {@link Model#serialize serializing} the model.
    */
   hasOne(Target, foreignKey, foreignKeyTarget) {
     return this._relation('hasOne', Target, { foreignKey, foreignKeyTarget }).init(this);
@@ -194,6 +197,9 @@ const BookshelfModel = ModelBase.extend({
    *   than `Target` model's `id` / `{@link Model#idAttribute idAttribute}`.
    *
    * @returns {Model}
+   *
+   *   The return value will always be a model, even if the relation doesn't exist, but in that
+   *   case the relation will be `null` when {@link Model#serialize serializing} the model.
    */
   belongsTo(Target, foreignKey, foreignKeyTarget) {
     return this._relation('belongsTo', Target, { foreignKey, foreignKeyTarget }).init(this);

diff --git a/lib/relation.js b/lib/relation.js
@@ -329,9 +329,7 @@ const Relation = RelationBase.extend({
 
   // Fetches all `eagerKeys` from the current relation.
   eagerKeys(response) {
-    const key = this.isInverse() && !this.isThrough()
-      ? this.key('foreignKey')
-      : this.parentIdAttribute;
+    const key = this.isInverse() && !this.isThrough() ? this.key('foreignKey') : this.parentIdAttribute;
     return _.reject(_(response).map(key).uniq().value(), _.isNil);
   },
 

diff --git a/test/integration/output/Relations.js b/test/integration/output/Relations.js
@@ -379,9 +379,6 @@ module.exports = {
       result: {
         id: 3,
         name: 'backbonejs.org',
-        meta: {
-
-        },
         blogs: [],
         authors: []
       }
@@ -390,9 +387,6 @@ module.exports = {
       result: {
         id: 3,
         name: 'backbonejs.org',
-        meta: {
-
-        },
         authors: [],
         blogs: []
       }
@@ -401,9 +395,6 @@ module.exports = {
       result: {
         id: 3,
         name: 'backbonejs.org',
-        meta: {
-
-        },
         blogs: [],
         authors: []
       }

diff --git a/test/integration/relations.js b/test/integration/relations.js
@@ -91,6 +91,12 @@ module.exports = function(Bookshelf) {
           }).then(checkTest(this));
         });
 
+        it('does not load "hasOne" relationship when it doesn\'t exist (site -> meta)', function() {
+          return new Site({id: 3}).fetch({withRelated: ['meta']}).then(function(site) {
+            expect(site.toJSON()).to.not.have.property('meta')
+          });
+        });
+
         it('eager loads "hasMany" relationships correctly (site -> authors, blogs)', function() {
           return new Site({id: 1}).fetch({
             withRelated: ['authors', 'blogs']