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

Move level store exports to index.ts #477

Merged
merged 3 commits into from
Aug 24, 2023
Merged

Move level store exports to index.ts #477

merged 3 commits into from
Aug 24, 2023

Conversation

amika-sq
Copy link
Contributor

React Native currently cannot import MessageStoreLevel, DataStoreLevel, or EventLogLevel, as they are in a named export called ./stores.

React Native currently does not support named exports (but it is coming in a future update), so we have to manually import those classes from the dist/esm/src/index-stores directory. Example: decentralized-identity/web5-wallet#52 (comment)

This is not ideal, but really causes issues when other packages use the ./stores import. For example, in @frankhinek's agent refactor PR, we're using the /stores named export here: https://github.com/TBD54566975/web5-js/pull/168/files#r1298468034. React Native is not able to do this, causing the @web5/agent package to also be incompatible with React Native.

We have a couple of options here:

  1. Move the MessageStoreLevel, DataStoreLevel, and EventLogLevel exports back into the primary dwn-sdk-js package (what this PR is doing).
  2. Create a separate package just for the stores.

Perhaps there are more options than this, and I'd be open to other suggestions!

shamilovtim
shamilovtim previously approved these changes Aug 18, 2023
@thehenrytsai
Copy link
Contributor

@amika-sq, sorry that I am confused. The reason that we introduced named exports for Level implementations of stores and even took that path of less-convenient Dwn creation was precisely so that React Native can import the DWN SDK without Level stores, because I was told that Level isn't supported in React Native.

If a web5 library took a dependency on Level stores and blows up React Native, shouldn't the fix be not having level store as a dependency in its default export?

@amika-sq
Copy link
Contributor Author

MessageStoreLevel, DataStoreLevel, and EventLogLevel themselves do not directly use Level. Instead, it interacts with LevelWrapper, which is an AbstractLevel

Only the default behavior for createLevelDatabase here directly interacts with Level.

React Native definitely supports abstract-level, as it's abstract. That's what is allowing us to provide an override for the createLevelDatabase, which creates an AbstractLevel instances that is compatible with React Native (example here).

@thehenrytsai
Copy link
Contributor

@amika-sq thanks for the clarification, so to summarize and make sure my understanding is correct:

MessageStoreLevel, DataStoreLevel, and EventLogLevel are not inherently incompatible with React Native, it's the default params to createLevelDatabase which creates an browser version of (but can be overridden) that is incompatible with React Native.

Follow up question:
Why is Level DB chosen? Level DB backed store was implemented because that's the only choice for browsers, I would have thought other types of DBs with better query capabilities such as SQL DB be better choices.

@amika-sq amika-sq merged commit 285036b into main Aug 24, 2023
@amika-sq amika-sq deleted the amika/level-stores branch August 24, 2023 13:27
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

Successfully merging this pull request may close these issues.

3 participants