This is an intentional breaking change to bring getIn and setIn into alignment. Bug's resulting from the silent return of null for this case were a common complaint from the previous major version.

The reason this fails is because there is a value at "a" however that value is not a container and doesn't allow deeper retrieval. This is usually indicative of a mistake, expecting the value at "a" to be a collection if it exists.

Note if you change the test code to:

const ABRecord = Immutable.Record({ a: null })
const myRecord = new ABRecord()
console.log(myRecord.getIn(['a', 'b']));

That you'll see null logged.

@leebyron Thanks for the detailed explanation, however I believe there is still a bug as the code you posted:

const ABRecord = Immutable.Record({a: null});
const myRecord = new ABRecord();
console.log(myRecord.getIn(['a', 'b']));

Also returns:

TypeError: Invalid keyPath: Value at ["a"] does not have a .get() method: null

Additionally, even if this is still intended behavior, I would highly recommend this breaking change get called out specifically on the 4.0 changelog as it was not clear from the current description and it's caused a lot of headaches for us during the upgrade process.

Ah interesting! I'll definitely make sure there's more tests and clear language around this

Reopening as a reminder to myself to dig in

It also happens on nested Map structure not only on Record.

@leebyron would it be possible to have this behavior have a deprecation cycle for one major version where instead of throwing a warning is logged? For example on our codebase we have 1000+ callsites of getIn to migrate before we can update.

Having a deprecation period with a warning for this will allow us to upgrade sooner and migrate the necessary codepaths with more confidence.

Same problem with hasIn

(Immutable.Map({"sdfsfddfs": null})).hasIn(["sdfsfddfs", "sdfdfs"]);

Uncaught TypeError: Invalid keyPath: Value at ["sdfsfddfs"] does not have a .get() method: null

I hope hasIn will keep his behavior....

Will be fixed in the next rc

Thanks! This is indeed fixed in 4.0.0-rc.4, however, now I'm getting this warning:

Invalid keyPath: Value at ["a"] does not have a .get() method: a
This warning will throw in a future version

Does this mean it will break again in a future version? The handling of nulls is an incredibly useful feature of getIn as:

const ABRecord = Immutable.Record({a: null});
const myRecord = new ABRecord();

// Without getIn you have to manually do this for all deeply nested keys
const a = myRecord.get('a');
const b = a ? a.get('b') : null;
const c = b ? b.get('c') : null;

// Whereas with `getIn` as it works now, it's super easy
const c = a.getIn(['a', 'b', 'c']);

Just wanted to get some feedback on the change and what the migration plan is. I'm very curious as to the reasons why this is planned for deprecation.

The warning is the migration plan. I can’t speak to why it’s planned for deprecation, but this warning should tell you all the callsites to fix before Immutable 5..

I'm confused as to the reason this is getting deprecated.

There's existing precedent in the JS community that getIn type methods allow you to go past defined values.

// lodash
const record = { name: { first: 'John', last: 'Brown' } };
_.get(record, 'name.first'); // John
_.get(record, 'name.first.last'); // undefined

Otherwise, as it stands in v4 I see little use for getIn. The use case seems to be entirely limited to records where the path of keys are all required. If there're any nullable values in the path, getIn is going to fail.

That requires that all deep lookups use chained gets, which seems to eliminate a use for getIn

const record = { a: { b: { c: [{ x: { y: 1 } }] } } };

// currently
const simple = record.getIn(['a', 'b', 'c', 0, 'x', 'y']);

// v4
const complex = record
  .get('a', Immutable.Map())
  .get('b', Immutable.Map())
  .get('c', Immutable.Map())
  .get(0, Immutable.Map())
  .get('x', Immutable.Map())
  .get('y', Immutable.Map())

Chaining gets with defualt values (or something along that line) is much less elegant. And, in the end, is basically manually performing the previous getIn functionality.

I would also think that this approach would open devs up to the potential for errors. For example, right now you can do getIn and define a default value if none is found, allowing you to handle undefined responsibly. However, with the future release, if you erroneously use getIn where a value along the path is nullable, you could experience failures before the default value is triggered.

I hope there's a way around this, because as it stands, the site I maintain would be quite adversely impacted by the removal of the v3 getIn null handling.

EDIT:

This is an intentional breaking change to bring getIn and setIn into alignment. Bug's resulting from the silent return of null for this case were a common complaint from the previous major version.

For me and my team, the bigger confusion that sometimes arose was this functionality:

const user = { name: { first: undefined: last: null } }; // gotten from an immutable redux store, perhaps

const first = user.getIn(['name', 'first'], 'John'); // Set default 
const last = user.getIn(['name', 'last'], 'Doe'); // Set default 

console.log(`User's name is: ${first} ${last}`);
// Result: "User's name is undefined null"
// Expected: "User's name is John Doe"

It seemed like a bug at first, since we expected the notSetValue to be returned, since the value was undefined or null, but we eventually understood that since the key was found in the path, notSetValue wasn't returned.

But, we rarely fell into situations where "the silent return of null" was causing issues, since we were typically good about defining default values. The bigger issue was needing to do something like this:

const user = { name: { first: null } };

const username = user.getIn(['name', 'first'], 'John'); // this is no good, will return `null`
const better = user.getIn(['name', 'first']) || 'John'; // Returns "John" as expected

Basically, sometimes we had to use || instead of notSetValue depending on our API's return data and model

Hey @Kyle-Mendes

I think you misunderstand the change. If you test this in v4, you'll find that getIn() with a deeper than existing key will continue to behave as it did before, and notSetValue can be provided to explicitly detect this case separately from a key path that does exist, but just happens to be set to null or undefined.

What this change regards is a getIn() path where the path encounters a value which does not support getting - this is almost always indicative of programmer error, which is why we sought to produce early error messages

Hey @leebyron

What this change regards is a getIn() path where the path encounters a value which does not support getting

As I understand this change, that means that the following is true, right?

Attempting to use getIn(['a', 'b', 'c']) where b === null will result in a warning being thrown, since null does not have a .get method

If this is true, then I disagree with the statement:

this is almost always indicative of programmer error,

In my experience, it is not uncommon for an API to return null for data that could be an object or array. For example, a user with no pets might have pets: null returned from the API.

take this example:

const DB = { users: [{ name: 'Kyle', pets: null }] };

function getUserPet(userIndex, petIndex) {
  return DB.getIn(['users', userIndex, 'pets', petIndex, 'name'], 'no pet');
}

getUserPet(0,0); // throws warning -- Value at ["pets"] does not have a .get() method: null

This, to me, is a real life example that isn't indicative of programmer error necessarily. I don't think it is uncommon for an API to return null for data that can be nested, so null handling with getIn was a clean way to handle those cases. In Immutable 3 this was desired functionality for me. I could attempt to deeply grab data, and if I encountered a null value I could handle it. All in a single line of a single function call (.getIn).

Now, as I mentioned above, this is much harder to do, and I don't see the gain.

I created a simple codepen to show you what I'm encountering: https://codepen.io/Pink401k/pen/NayNPp

You can see what I mean in the console. I also included the current Immutable 3 CDN to illustrate that no errors are thrown currently.

Thanks for working through this with me. I just hope to illustrate that the current v3 functionality is desirable (at least in some circumstances). I hope that there's a way to keep null handling in v4 and beyond. If the Immutable team would really like to deprecate the current functionality, perhaps a new method getDeep or a flag like ignoreNull could be passed to allow developers to work safely within the new system.

As I understand this change, that means that the following is true, right?

No not that is not the intention as I stated. Please open a bug issue

@leebyron Will do, thanks again :)