Allow plain objects in Custom loadSession method

[thecodepixi]

WHY are these changes introduced?

Previously, we always expected the return from the loadCallback to already be an instanceof Session, but this can cause some issues in developing the loadCallback method for a custom storage solution, due to having to do some extra object manipulation.

WHAT is this pull request doing?

CustomSessionStorage.loadSession will now allow the loadCallback to return either an instanceof Session or a plain object. If a plain object is returned, it will be used to instantiate a new Session which will then be returned.

Type of change

• Patch: Bug (non-breaking change which fixes an issue)
  - [ ] Minor: New feature (non-breaking change which adds functionality)
  - [ ] Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

• I have added a changelog entry, prefixed by the type of change noted above
• I have added/updated tests for this change
• I have documented new APIs/updated the documentation for modified APIs (for public APIs)

NOTE: I have updated the type on the current CustomSessionStorage docs under issues.md, but will be adding to and updating this docs for CustomSessionStorage more thoroughly in a separate PR.

[paulomarg]

I like this but I wonder if it wouldn't be simpler to simply take in the JSON string? We could allow both but that seems a bit much.

What do you think?

[thecodepixi]

I'm not sure I have strong feeling for or against that, it would just require more checking. One thing it does raise in my mind is that we're maybe reaching into their storage implementation by doing that? The custom storage solution should handle its own data in the loadCallback and then give it to us to essentially "pass through" loadSession. By offering to manipulate stringified objects (or potentially non-parseable strings) we're combining their solution with our solution, in a way. But if parsing doesn't work, that's their own error, not ours.

[paulomarg]

That's fair - at some point we have to translate the external object to ours, but it does make it more flexible if we don't assume it will be JSON.

Even though I suspect JSON will be used in the vast majority of the cases, there's no reason not to support other methods as well.

[thecodepixi]

there's no reason not to support other methods as well.

I can't tell if we're on the same page here? Do you mean we should add support for further data types in this method? My philosophy is "please make your data into this shape first" and have them pass in at least a JSON object. I think the manipulation of their data should be more delegated to their custom callback than to the library, personally. But since the library is what stipulates "make this a Session" it makes sense that we handle that step for them.

[paulomarg]

No, I was agreeing with you - as long as they give us an object that maps to what our session looks like, apps can do whatever they want with their objects. By 'other formats' I meant in how they would possibly serialize and de-serialize the object to store it in their database (like say a MongoDB object that's NOT converted to JSON, or something).

[thecodepixi]

Ah ok yes! Yeah, I think this way we're being as flexible as possible, because we don't really care what they do to store/access the data, as long as it comes back to us in the right shape.

[paulomarg]

Minor nits but I'm good with it being merged as is.