Allow returning promises from StorageManager `.load` and `.store` methods

[tom-sherman]

This would offer a more modern and clean API than the current callback based approach.

Proposed API:

editor.StorageManager.add('simple-storage', {
  load: async keys => {
    const result = await foo(keys)
    return JSON.parse(result)
  },
  async store(data) {
    const result = await bar(data)
    return result
  }
});

---

There are a few ways this could be implemented:

1. Detect if the load/save method is thenable, if so await

This would complicate the API and code a little as users would need to make a choice between using either the callback or Promise based approach. The upside to this though is that it would not be a breaking change.

A side effect of this API change would allow mixing and matching of approaches:

// An async load method and callback store method
editor.StorageManager.add('simple-storage', {
  load: async keys => {
    const result = await foo(keys)
    return JSON.parse(result)
  },
  store(data, clb, clbErr) {
    bar(data, err => {
      if (err) return clbErr(err)
      clb()
    })
  }
});

2. Switch to a Promise only StorageManager API

The big benefit of this is we would be encouraging a more best practice approach and the API would be far less complicated. Not having to support two slightly different APIs is also good for maintenance. Unfortunately it would be a breaking change and might be best paired with a Promise rollout across the whole library. There are other APIs that could do with supporting promises too, commands for example.

3. Add new methods asyncLoad and asyncStore

The user would need to choose to either use the callback-based load/store methods, or their promise-based async counterparts. To me, this seems like the worst of options 1 & 2.

I'd be happy to open a PR for this, but would be good to get thoughts on which approach is best before continuing :smile:

[artf]

Really cool feature request Tom, I'd totally appreciate a PR.

For sure I'd go with your first approach, not breaking the current API is always a goal to reach. Just adding the ability to create async-await enabled custom storage managers would be amazing. Later we might also update the built-in local and remote storages to work in that way.

Here an example of what I'd expect to work (to check also if storage events work correctly)

    const asyncStorage = {};

    editor.StorageManager.add('async-storage', {
      async load(keys) {
        return new Promise(res => {
          setTimeout(() => {
            res(keys.reduce((acc, key) => {
              acc[key] = asyncStorage[key];
              return acc;
            }, {}))
          }, 1000);
        });
      },
      async store(data) {
        return new Promise(res => {
          setTimeout(() => {
            for (let key in data) {
              asyncStorage[key] = data[key];
            }
            res();
          }, 1000);
        });
      }
    });

This would complicate the API and code a little as users would need to make a choice between using either the callback or Promise based approach

A side effect of this API change would allow mixing and matching of approaches

I'd just care about updating the documentation here https://grapesjs.com/docs/modules/Storage.html#define-new-storage In this way, publicly, only the async-await will be visible

[tom-sherman]

Thanks for your thoughts! Hopefully I'll have time to implement it this week

[artf]

With the new StorageManager refactor this can be closed https://github.com/artf/grapesjs/pull/4223