-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Data: clean up registerGenericStore param names #36300
Conversation
Size Change: +6 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
unsubscribeFromRootStore(); | ||
} | ||
unsubscribeFromStoreEmitter(); | ||
unsubscribeFromStore?.(); |
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.
do we have this in our Babel transforms? I didn't realize we could use optional-chaining in the JavaScript code
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.
The support was added via #19831.
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.
There's also plenty of other places in the codebase where ?.
is already used.
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.
Thanks for taking this one. Renaming key
was one of the things I planned on doing after some other updates anyway, so I appreciate that I won't have to do that now.
We have other uses of key
, such as in instantiateReduxStore
, but I think it's fine to tackle those in separate work. That function also calls it a "unique namespace identifier," and I had wondered if we might review the use of name
vs namespace
.
All things here look like positive changes 👍
Related to #36190 -- improves param names of the
registerGenericStore
method. The second parameter is not aconfig
, but a store instance, so I call itstore
. Naming the first parameter asname
instead ofkey
is also more consistent with the rest of the codebase.How to test:
Shouldn't change any behavior, it's just renaming params and variables.