rtfeldman/seamless-immutable

isMergableObject returns true for an empty object

Open

#82 opened on 2015年12月28日

GitHub で見る
 (1 comment) (0 reactions) (0 assignees)JavaScript (5,367 stars) (221 forks)batch import
bughelp wanted

説明

      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.

コントリビューターガイド

技術スタック
javascript
領域
developer experience
Issue 種別
bug
難度新しい貢献者にとっての実装難度です。1 は非常に小さな変更、5 は専門的な作業です。
2
推定時間経験ある貢献者が調査、実装、テスト、pull request 準備にかけるおおよその時間範囲です。
under 1 hour
活動状況Issue が今どれだけ取り組みやすいかを示します。新しい、活発、古い、ブロック中、またはメンテナー入力待ちなどです。
stale
明確さ期待される変更、受け入れ条件、次の手順がどれだけ明確かを示します。
clear
前提条件
JavaScript basicsimmutable data concepts
初心者向け度初回貢献者にどれだけ取り組みやすいかを 1-100 で推定したスコアです。
80
調査方針
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.
isMergableObject returns true for an empty object · rtfeldman/seamless-immutable#82 | Good First Issue