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

Add adapter key prefix #28

Closed
yumauri opened this issue Sep 16, 2021 · 5 comments
Closed

Add adapter key prefix #28

yumauri opened this issue Sep 16, 2021 · 5 comments
Labels
help wanted Extra attention is needed rfc Request for comments

Comments

@yumauri
Copy link
Owner

yumauri commented Sep 16, 2021

Let's say you have two separate applications on the same domain

As long as they are located on the same domain — they shares same local storage space. And this can lead to keys conflicts. For example, both applications has store $isAuthenticated and both applications persists this store with the key authenticated in storage.

It could be hard to maintain unique keys manually, so, idea is to add possibility to specify global (per adapter) key prefix. For example:

import { createStore } from 'effector'
import { persist, configure } from 'effector-storage/local'

configure({
  keyPrefix: 'app1/'
})

const $isAuthenticated = createStore(false)
persist({
  store: $isAuthenticated,
  key: 'authenticated' // <- actual localStorage key will be "app1/authenticated"
})

(This API example is just an idea and subject to change)

@yumauri yumauri added help wanted Extra attention is needed rfc Request for comments labels Sep 16, 2021
@AlexandrHoroshih
Copy link

AlexandrHoroshih commented Sep 16, 2021

The global configuration tricks look very suspicious to me

Maybe something like this?

const appPersist = createPersist({
 prefix: 'app1/',
 //  more configuration options can be added in the future, if needed
})

const $isAuthenticated = createStore(false)
appPersist({
  store: $isAuthenticated,
  key: 'authenticated' // <- actual localStorage key will be "app1/authenticated"
})

// this way we can also persist a value, that is shared for both apps, if we need this
persist({
 store: $shared,
 key: 'shared-value'
})

@yumauri
Copy link
Owner Author

yumauri commented Sep 16, 2021

Hm, I agree, this looks more robust.

But it is always irritates me in ky that if I want to change some options globally — I need to create separate instance of ky and use it everywhere across the application...

What about both ways?
For example axios supports changing global defaults and creating separate instance with its own defaults:
https://github.com/axios/axios#config-defaults

@yumauri
Copy link
Owner Author

yumauri commented Sep 16, 2021

Taking axios behaviour as an example:

Global configuration:

persist.defaults.keyPrefix = 'app/'

const $isAuthenticated = createStore(false)
persist({
  store: $isAuthenticated,
  key: 'authenticated' // <- actual localStorage key will be "app/authenticated"
})

Instance configuration:

const appPersist = persist.create({
  keyPrefix: 'app/',
})

const $isAuthenticated = createStore(false)
appPersist({
  store: $isAuthenticated,
  key: 'authenticated' // <- actual localStorage key will be "app/authenticated"
})

// defaults can be changed on instance as well
appPersist.defaults.keyPrefix = 'app2/'

@AlexandrHoroshih
Copy link

AlexandrHoroshih commented Sep 17, 2021

I don't know, i really don't like global configration hooks
🤷

What is correct behavior, if user changed global configuration long after all persists are set? or if global configuration changes on the fly?

Maybe forbid to change configuration if there are persisted stores already, or allow to call this configure function only one time? 🤔

@yumauri
Copy link
Owner Author

yumauri commented Sep 28, 2021

After several days of thinking I have come to conclusion, that you are right — global configuration will add useless complexity and strange behaviour.

Added createPersist factory in version 4.4.0.

@yumauri yumauri closed this as completed Sep 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed rfc Request for comments
Projects
None yet
Development

No branches or pull requests

2 participants