Recoil icon indicating copy to clipboard operation
Recoil copied to clipboard

Why doesn’t Recoil manage the unique key in atom and selector

Open mdovn opened this issue 5 years ago • 15 comments

I don’t see that the user needs to use the defined key anywhere. So why does he/she needs to create it manually ?

const textState = atom({
  key: 'textState', // this is used nowhere, why not just abstract it out?
  default: '', 
});

mdovn avatar Jun 22 '20 18:06 mdovn

See https://github.com/facebookexperimental/Recoil/issues/32#issuecomment-629403087

Having a string key is necessary to provide a fixture for state which survives past the lifetime of the app -- it enables scenarios such as debugging for dev tools, and stable persistence.

A question some of our team members are curious about: how burdensome is it to add keys?

mondaychen avatar Jun 23 '20 17:06 mondaychen

Thanks @mondaychen for clarifying. Yeah, but it’s really burdensome to manage the uniqueness of keys. Since it’s good only for a few use cases, I suggest making it optional. I’m not sure if it’s a good idea but just a thought.

mdovn avatar Jun 23 '20 18:06 mdovn

@mdovn You could always use a library like uuid to handle the uniqueness for you.

This works:

import { v4 as uuidv4 } from "uuid";

export const myAtom = atom({
	key: uuidv4(),
	default: null,
});

DynamicArray avatar Jun 24 '20 11:06 DynamicArray

...although I haven't found creating key names a burden at all :)

DynamicArray avatar Jun 24 '20 11:06 DynamicArray

See #32 (comment)

Having a string key is necessary to provide a fixture for state which survives past the lifetime of the app -- it enables scenarios such as debugging for dev tools, and stable persistence.

A question some of our team members are curious about: how burdensome is it to add keys?

My team is looking into adopting Recoil for a project, but since we expect the project to grow and stay around for a long time, we're very concerned about key collisions happening. It seems like a landmine whose surface area gets bigger and bigger the more you work on an app. Maybe generate a key internally as a fallback, but have tutorials and documentation strongly encourage adding an optional name value to the atom definition?

dkovacevic15 avatar Jun 24 '20 14:06 dkovacevic15

See this issue, when #56 (which should probably still be open) is implemented this may be a possibility. I don't know if they'll go for it, but that's a blocker atm.

Shmew avatar Jun 24 '20 14:06 Shmew

hi, im using recoil lib custom compiled for react native with some helper:

import 'react-native-get-random-values'
import { v4 as uuidv4 } from 'uuid'
import * as recoil from './original'

const { atom: recoilAtom, ...rest } = recoil

function atom(defaultValue) {
  const uuid = uuidv4()
  return recoilAtom({
    key: uuid,
    default: defaultValue
  })
}

module.exports = {
  atom,
  ...rest
}

two weeks, working fine atoms created only on start of application and not re-initialized during app lifetime

dont understand from where 'memory-leak' can be... if not create them inside dynamic functions... but then we cannot import them...

leeratyou avatar Jun 24 '20 18:06 leeratyou

naming is really hard.
since key can be generated automatically using tools like uuidv4(), I want Recoil to handle the task for me.
Human is unreliable, easy to make names conflict.
please save me from naming.

uptonking avatar Jul 06 '20 03:07 uptonking

I think the issue here is that many of us don't care about persisting our recoil state. For that use case there's really no need for us as users to think about these unique IDs, they're just boilerplate + a footgun.

pelotom avatar Jul 17 '20 22:07 pelotom

hi, im using recoil lib custom compiled for react native with some helper:

import 'react-native-get-random-values'
import { v4 as uuidv4 } from 'uuid'
import * as recoil from './original'

const { atom: recoilAtom, ...rest } = recoil

function atom(defaultValue) {
  const uuid = uuidv4()
  return recoilAtom({
    key: uuid,
    default: defaultValue
  })
}

module.exports = {
  atom,
  ...rest
}

two weeks, working fine atoms created only on start of application and not re-initialized during app lifetime

dont understand from where 'memory-leak' can be... if not create them inside dynamic functions... but then we cannot import them...

This way may cause some problems such as cache, when you refresh page, will uuid change?

nanslee avatar Dec 25 '20 09:12 nanslee

https://github.com/facebookexperimental/Recoil/issues/378#issuecomment-751221106

This way may cause some problems such as cache, when you refresh page, will uuid change?

Still, this is an issue only if the state persists through page reloads. I'd argue that most people are not interested in this scenario (based on the situation in other state libs, i.e. redux which has 4.4kk weekly downloads vs redux-persist with 400k – around 10% of the users being interested in persistence).

https://github.com/facebookexperimental/Recoil/issues/378#issuecomment-648314856

it enables scenarios such as debugging for dev tools, and stable persistence.

Those seem like opt-in features – if Recoil doesn't use those keys for anything internally then I don't see a reason to generate them at all (even automatically by Recoil itself) if the user doesn't explicitly ask for it 🤔 (i.e. by passing in a debug flag to Recoil, which would cause it to generate keys on its own if one hasn't been defined by the user).

For persistence it seems like there is no other option than to force the user to define the keys (to avoid the aforementioned problem of uuids changing between reloads).

kdmadej avatar Jan 12 '21 11:01 kdmadej

As a new user, I don't find it cumbersome at all to use, simply a bit odd (I agree with OP, anything that can be automated should be) and confusing.

I start asking myself "Mmmm... why are they asking me to set a unique key, what am I missing? WHY?? Are they used by selectors perhaps ... ?" and then went on a wild chase trying to understand why these keys are not handled internally. That's how I ended here. In the end it's not a big deal, but it's it lowers my confidence that I understand what is going on, as well as the minor time waste.

A simple remark in the key section in the atom page of the documentation would be helpful. Would have opened a PR myself but it doesn't seem to be in this repo

gotofritz avatar Apr 04 '21 20:04 gotofritz

Please save me from naming +1 😂

mister-teddy avatar Apr 10 '21 18:04 mister-teddy

A good intermediate option is https://github.com/dsherret/ts-nameof/tree/master/packages/ts-nameof.macro it is a babel macro that takes a function name as an option


export const formatRepetitionForDisplaySelector = selector({
   key: nameof(formatRepetitionForDisplay),
   get: ({ get }) => {
      return formatRepetitionForDisplay(get(intlProviderAtom), get(initativeRecurrenceAtom));
   },
});

ShravanSunder avatar Apr 17 '21 12:04 ShravanSunder

There is something called jotai. It does not use keys by default. It needs keys for persistence via an explicit atomWithStorage atom creator. It needs keys to not loose state during React fast refresh, those keys can be automatically added via a Babel transform.

king-of-poppk avatar Aug 02 '23 08:08 king-of-poppk