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

🔷 [Epic] Persistent siloed browser localstorage & session storage #27

Closed
mpeterdev opened this issue Aug 11, 2023 · 9 comments · Fixed by #417
Closed

🔷 [Epic] Persistent siloed browser localstorage & session storage #27

mpeterdev opened this issue Aug 11, 2023 · 9 comments · Fixed by #417

Comments

@mpeterdev
Copy link
Collaborator

Components should be able to store and retrieve data from browser storage across sessions

@mpeterdev mpeterdev changed the title Persistent siloed browser storage 🔷 [Epic] Persistent siloed browser storage Oct 10, 2023
@mpeterdev mpeterdev added the Epic label Oct 10, 2023
@mpeterdev
Copy link
Collaborator Author

Andy Haynes

is the state management epic orthogonal to the browser storage from #27? #18

just curious if something like a React Query implementation would satisfy both requirements

Michael Peter

thinking :loading-wheel:

I think simple browser storage merits its own implementation

there are a lot of use cases for localstorage/sessionstorage that developer may want to quickly implement without learning the full patterns for global state management stores

there should certainly be some degree of overlap though. Global stores being persisted to browser storage is valuable

FYI React-Query is not persistent by default, its just an in memory cache

Andy Haynes

Yeah I guess the “persistent” part is key, though that could just be hydrated by a query layer

I’m just wondering it makes sense to build this cache with the intention of extending it later to support a more unified API for fetching data external to the component. Nothing we need to flesh out now, just something to think about while we’re building it

Michael Peter

yeah we should bring that up but I think the answer will end up being that it’s not worth designing for that until we’re ready for the full implementation. #27 will likely yield a very light wrapper over browser storage which really is only responsible for namespacing data by component

i.e. calling localStorage.setItem("foo", "bar"); in the component actually yields bwe-demos.near/ExampleComponent/foo:'bar' in localstorage

Andy Haynes

we’re not going to shadow localStorage though right? 😅

Michael Peter

what do you mean by shadow?

Andy Haynes

override it so that get/sets actually set something in the outer application storage

like we’ll have our own method for it that’s not called localStorage.setItem

Michael Peter

I had been assuming we would do that but I do not feel strongly that we should. why does that come off as undesirable to you?

Andy Haynes

the methods would become async and you could no longer set localstorage within your own component container iframe

it would just break any legit localstorage usage within a Component

Michael Peter

ah I see, tbf I hadn’t even considered how storage would work inside iframes

would it really work anyways though? on subsequent loads is the same component going to get access to the previous instance of the container’s localstorage?

I hadnt seen anything about how that would be possible with srcdoc

Andy Haynes

no I think you’re right that it would be ephemeral, but the author may have some reason to do it or rely on a package that does

I doubt there’s a non-convoluted use case for it

Michael Peter

fair. I still see your point and not shadowing is probably the correct move. Maybe just a similar name with the same API (or as close as we can get)

i.e. componentStorage.setItem

Andy Haynes

for me it just reinforces the need to park these implicit methods under an explicit namespace, probably in the iframe’s window object

Michael Peter

I think you’re right, especially with the proper type definitions that you have an object which you can hover in your IDE to see everything available

@mpeterdev mpeterdev changed the title 🔷 [Epic] Persistent siloed browser storage 🔷 [Epic] Persistent siloed browser localstorage & session storage Mar 12, 2024
@mpeterdev
Copy link
Collaborator Author

Originally this was intended to replicate BOS VM functionality where every instance of the same component shares local storage. This is likely not ideal for real world usage. Instead, the scope should be more granular, perhaps container ID + component key

@mpeterdev
Copy link
Collaborator Author

since this is going to result in an async API anyways, it's worth considering backing this by IndexedDB instead of localstorage

@mpeterdev
Copy link
Collaborator Author

not in scope but may be requested in future: https://developer.mozilla.org/en-US/docs/Web/API/Window/storage_event

@mpeterdev
Copy link
Collaborator Author

mpeterdev commented Mar 19, 2024

Goal

supports standard localstorage use cases

Considerations

data needs to be protected from arbitrary access by other components on the page

Implementation notes

  • prefix : container ID combined with subcomponent identifier
  • multiple instances of the same component do not share storage in this version. That would be a separate future feature
  • changing key of component performing storage or anywhere in upwards tree will cause loss of storage contents because the identifier changes
  • will be backed by indexeddb and can deal directly in objects instead of requiring JSON stringify+parse manually at component level
  • might need to define TTL on indexeddb: future scope

@mpeterdev
Copy link
Collaborator Author

figjam

@pavelisnear
Copy link
Contributor

pavelisnear commented Apr 10, 2024

Leaving a few thoughts and open questions on the Storage implementation.

Storage will be referred as a RocStorage, but in general it will mimic the browser LocalStorage.

1. API assumptions:

In order to send the message from the component to the RocStorage, we need to provide a way to access its api from the user component.
Might be something like an update in SandboxedIframe.tsx

const rocStorage = /* implementation of the storage */

This way we can inject the rocStorage in the global scope:

export default function () {
  console.log(rocStorage) // contains the rocStorage from the SandboxedIframe
  return (
    <div>
      <h1>Storage test</h1>
    </div>
  );
}

might be good to "shadow the localStorage" to have IDE support, etc.

2. Transport assumptions:

The component can not directly read/write to the browser Local Storage, that's why the transport of the api call should be done via the .postMessage calls.

  • should we use a separate message listener for this goal (not compiler)?
  • a unique component signature to restrict the data access
  • define data clean up approach

3. Component folders:

  • Since for every component fetched via rpc request we have a block height, should we use it in the storage path concatenated with the component path?

@andy-haynes
Copy link
Contributor

This way we can inject the rocStorage in the global scope

I think we should avoid global scope injection wherever possible - it's abused enough as is 😅. Maybe an RoC plugin based approach would work like we do with @bos-web-engine/wallet-selector-plugin? Then we would publish a @bos-web-engine/container-storage package that Components import directly. The plugin architecture then allows Components to use Container function calls to invoke methods safely in the outer application.

The component can not directly read/write to the browser Local Storage, that's why the transport of the api call should be done via the .postMessage calls

This is what the plugin architecture would get us for free 🙂

Check out the Wallet Selector RoC plugin here: https://github.com/near/bos-web-engine/blob/main/packages/wallet-selector-plugin/src/index.ts

And how the outer application handles plugin methods: https://github.com/near/bos-web-engine/blob/main/packages/application/src/handlers.ts#L14

@pavelisnear
Copy link
Contributor

Summary from our R&D discussion:

  1. Stick to the plugin approach proposed by @andy-haynes
  2. Do not focus on the storage clean up for now
  3. Use the component path - similar to the existing plugins (no need to add or remove block id)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Status: Done
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants