rtfeldman/seamless-immutable

isMergableObject returns true for an empty object

Open

#82 ouverte le 28 déc. 2015

Voir sur GitHub
 (1 commentaire) (0 réactions) (0 assignés)JavaScript (5 367 stars) (221 forks)batch import
bughelp wanted

Description

      function addToResult(currentObj, otherObj, key) {
      var immutableValue = Immutable(otherObj[key]);
      var mergerResult = merger && merger(currentObj[key], immutableValue, config);
      var currentValue = currentObj[key];

      console.log("IS NEW OBJECT MERGABLE", isMergableObject(new Object()));

  -> seamless-immutable.js:306 IS NEW OBJECT MERGABLE true

In my production build currentValue is sometimes an empty object, and with deep merge enabled we error out when trying to call merge() on said object. In dev mode this never seems to happen.

I've worked around this with an additional check to isMergableObject which checks that Object.keys(target).length > 0. If this seems like an acceptable fix I'd love to submit a PR.

Guide contributeur

Stack technique
javascript
Domaine
developer experience
Type d'issue
bug
DifficultéDifficulté estimée pour un nouveau contributeur, de 1 pour un très petit changement à 5 pour un travail expert.
2
Temps estiméFourchette de temps approximative pour investiguer, implémenter, tester et préparer une pull request.
under 1 hour
Statut d'activitéDisponibilité apparente de l'issue : fraîche, active, ancienne, bloquée ou en attente d'un mainteneur.
stale
ClartéClarté avec laquelle l'issue explique le changement attendu, les critères d'acceptation et la prochaine étape.
clear
Prérequis
JavaScript basicsimmutable data concepts
Accessibilité débutantScore de 1 à 100 estimant l'accessibilité de cette issue pour un premier contributeur.
80
Direction de recherche
Examine the `isMergableObject` function in `seamless immutable.js` around line 306. The proposed fix is to add a check that `Object.keys(target).length > 0` to return false for empty objects. Ensure to update any existing tests or add a new test for this edge case. Also investigate why the issue only occurs in production builds; it might be related to JavaScript engine optimizations.