In the implementation of Storage.has(key), we should check against null instead of undefined to see if a key exists in the storage.

[yutao-huang]

According to the W3C standard (https://www.w3.org/TR/webstorage/#dom-storage-getitem):

... If the given key does not exist in the list associated with the object then this method must return null.

In the existing code, this.get(key) !== undefined would always be true even when the key doesn't exist. In this case, this.get(key) actually returns null.

This incorrect behavior is also impacting things like authenticator.endpoints.has(...), which always mistakenly returns true for an actually not registered endpoint.

[yutao-huang]

Adding the related issue: #126

[Zlatkovsky]

Should it check for both null and undefined, for safety?

[yutao-huang]

@Zlatkovsky thanks for the approval!

I've verified the behavior of localStorage.getItem on Chrome/Edge/FireFox/Safari/IE and all returns null for non-existent keys. Wondering what could be the situation that would make it return undefined. (I did not check for older versions, though. I assume the behaviors of the browsers should not have changed...)

[Zlatkovsky]

@yutao-huang , it comes down to implementation details. For example, depending on whether the underlying implementation uses localStorage[xyz] syntax or localStorage.getItem(xyz) syntax, you get two different results!

I expect that's how the bug found its way here in the first place -- that Bhargav had originally had one implementation, then changed it out for another, and suddenly an older assumption about undefined was no longer true.

While null and undefined are semantically different, I would argue that they are mostly the same for purposes of this class. So I think it's better to check for both rather than have it only do one, and then risk an internal-implementation change breaking the outer contract.

[Zlatkovsky]

As for my approval -- note that you still will need someone else like @sumurthy to actually merge in the changes. I don't have any admin rights on this repo, so I don't know if my "approval" means much per se.

[yutao-huang]

@Zlatkovsky , gotcha, that makes sense. I added the check for safety. I'll ping @sumurthy to get some help. Thanks!