Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storing of Set object fails #97

Open
tcchau opened this issue Apr 19, 2021 · 5 comments
Open

Storing of Set object fails #97

tcchau opened this issue Apr 19, 2021 · 5 comments

Comments

@tcchau
Copy link

tcchau commented Apr 19, 2021

Per subject, if you try to store a Set object, the operation fails, storing an empty object instead.

For example:

    const colors = new Set();
    colors.add('blue');
    colors.add('red');
    lscache.set('colors', colors);
@peter-gribanov
Copy link
Contributor

peter-gribanov commented Apr 20, 2021

I guess the problem is that Set object is not serializable.

const colors = new Set();
colors.add('blue');
colors.add('red');

console.log(JSON.stringify(colors)); // {}

You will see a similar problem if you use the localStorage directly.

const colors = new Set();
colors.add('blue');
colors.add('red');

localStorage.setItem('colors', colors);
console.log(localStorage.getItem('colors')); // [object Set]

@tcchau
Copy link
Author

tcchau commented Apr 20, 2021

@peter-gribanov I can understand that. I'm currently using my own wrapper that detects a Set object and then converts it to an array for storage. I haven't had time to look at the standard to figure out whether the iterator order is guaranteed to be the same somehow when I convert the array back into a Set, but I'm guessing that will be the case.

@peter-gribanov
Copy link
Contributor

peter-gribanov commented Apr 21, 2021

I see a problem with unserializing Set object.
You are casting Set object to array. How are you going to restore this value?
Only you, as a user of this library and a developer of your application, know that under this key in localStorage, Set object serialized into array is stored and not a Array or a Map, WeakSet, WeakMap object serialized into an array.

We can add to this project the conversion of Set, Map, WeakSet or WeakMap objects to array, but when you read from storage, you will receive array, not Set, Map, WeakSet or WeakMap object. To restore Set, Map, WeakSet, WeakMap objects from array, we need to store meta information for each key. This will negatively affect performance and increase the amount of information in localStorage.

I think that solving such problems on the side of your application is easier and more efficient.

@tcchau
Copy link
Author

tcchau commented Apr 24, 2021

@peter-gribanov Certainly, it's up to the project maintainers to decide what is within the scope of the project. Presumably, metadata is only required when the object being cached is one of these objects that are not natively serializable, so the performance impact should not occur if they are not used.

I think perhaps a runtime check to ensure the user doesn't get something unexpected will be helpful. The type signature indicates that Object is supported so it's not unreasonable for a user of the library to assume that object serialization is taken care of by the library.

As I mentioned, I've already handled this case in my own application by writing wrappers.

@peter-gribanov
Copy link
Contributor

peter-gribanov commented Apr 28, 2021

I think perhaps a runtime check to ensure the user doesn't get something unexpected will be helpful. The type signature indicates that Object is supported so it's not unreasonable for a user of the library to assume that object serialization is taken care of by the library.

We expect that in localStorage, you will store simple data types: scalar values, arrays, or associative arrays (which are objects in JavaScripts). The annotations indicate Object as the type of stored values, but that does not mean that this library allows you to use any object.

console.log(JSON.stringify(new Date())); // "2021-04-28T10:17:36.626Z"
class Point {
  constructor(x, y) {
    this.x = x;
    this.y = y;
  }
}

console.log(JSON.stringify(new Point(100, 150))); // {"x":100,"y":150}
// result is equal for
console.log(JSON.stringify({x: 100, y: 150})); // {"x":100,"y":150}

We can serialize some of these objects, but we cannot always correctly restore these objects even knowing their type. The most we can do is detect an attempt to save an instance of a class. We'll end up with something like this.

const scalarTypes = ['undefined', 'boolean', 'number', 'string'];

function isSerializable(value) {
  if (scalarTypes.includes(typeof value)) {
    return true;
  }

  // i don't know how cross-browser this code is
  if (typeof value === 'object' &&
    value.hasOwnProperty('constructor') &&
    value.constructor.hasOwnProperty('name') &&
    value.constructor.name === 'Object' // is not an instance of some class
  ) {
    // all values of properties and nested properties is serializable
    for (const [k, v] of Object.entries(value)) {
      if (!isSerializable(v)) {
        return false;
      }
    }

    return true;
  }

  return false;
}

It seems to me that such a task is beyond the scope of this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants