-
Notifications
You must be signed in to change notification settings - Fork 244
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
Multi contract keystore #1369
Multi contract keystore #1369
Conversation
🦋 Changeset detectedLatest commit: 5148b67 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one CYA otherwise looking good.
Is it possible to retain the commit history so that Amir remains author on the parts he wrote? I can try to merge the original PR into a feature branch that you could rebase off of if that's easier.
constructor(localStorage: any = window.localStorage, prefix = LOCAL_STORAGE_KEY_PREFIX) { | ||
super(); | ||
this.localStorage = localStorage; | ||
this.prefix = prefix; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please make this this.prefix = prefix || LOCAL_STORAGE_PREFIX;
?
In the current implementation, an empty string prefix means clear()
would delete all localstorage entries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's merge the original PR into a feature branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to #1370 |
Pre-flight checklist
pnpm changeset
to create achangeset
JSON document appropriate for this change.Motivation
Test Plan
Added unit tests.
Related issues/PRs
#1163