Allow prefixes within keyMirror #4228
Conversation
Though this change may not exactly create “mirror,” I wanted to at least start a discussion around a problem that we have begun to run into when modules from separate domains listen for the same action and experience value collisions. For example:
```js
// modules/audio/constants/ActionTypes.js
var AudioPlayerActionTypes = keyMirror({
PLAY: null,
PAUSE: null
});
// modules/video/constants/ActionTypes.js
var VideoPlayerActionTypes = keyMirror({
PLAY: null,
PAUSE: null
});
```
Both an audio player and a video player would implement some type of play button within a UI layer that, when clicked, runs through an action creator that eventually reaches the Dispatcher and ultimately sends a `'PLAY'` action out into the parent application. If the media components are only listening for a `'PLAY'` action to be dispatched, *both* will unintentionally begin playback.
One solution would be to namespace keys in each constants file:
```js
// modules/audio/constants/ActionTypes.js
var AudioPlayerActionTypes = keyMirror({
AUDIO_PLAYER_PLAY: null,
AUDIO_PLAYER_PAUSE: null
});
// modules/video/constants/ActionTypes.js
var VideoPlayerActionTypes = keyMirror({
VIDEO_PLAYER_PLAY: null,
VIDEO_PLAYER_PAUSE: null
});
```
This works, but is overly verbose and repetitive in my opinion (`VideoPlayerActionTypes.VIDEO_PLAYER_PLAY`.) By adding a prefix option, the keys remain slim, with their namespace still denoted by the object they are members of, but avoids any value collisions.
|
I think @zpao or @spicyj are the ultimate authority here. That said, I personally don't see a problem with adding some sort of namespacing, though I'm not sure I really see the need (for reason below) and also I don't think the stuff in vendor is React public API, is it? Isn't it questionable to be using it for application code? More importantly, your solution is not a general one. What if you have two video players loaded on the same page? You need a way to differentiate not only between the TYPE of the event, but also which instance of the player. Therefore, it's probably better for your video player to have a unique internal key to identify the player, and include that key in your events, such that the event is always uniquely tied to the player that is being controlled. We still need to figure out a viable solution for SSR (#4000) but if you're just using React client side, a UUID is on the component instance probably what you're looking for. |
|
IIRC, the keyMirror module is not to be used externally. @restlessdesign you'd better create your own npm or local module for that. |
|
Yeah, we don't support any modules except the main React module and the addons. Feel free to make a fork (or rewrite) of keyMirror and publish it independently, like https://github.com/JedWatson/classnames was relative to |
|
it seems the examples are using https://www.npmjs.com/package/keymirror |
Though this change may not exactly create a “mirror,” I wanted to at least start a discussion around a problem that we have begun to run into when modules from separate domains listen for the same action and experience value collisions. For example:
Both an audio player and a video player would implement some type of play button within a UI layer that, when clicked, runs through an action creator that eventually reaches the Dispatcher and ultimately sends a
'PLAY'action out into the parent application. If the media components are only listening for a'PLAY'action to be dispatched, both will unintentionally begin playback.One solution would be to namespace keys in each constants file:
This works, but is overly verbose and repetitive in my opinion (
VideoPlayerActionTypes.VIDEO_PLAYER_PLAY.) By adding a prefix option, the keys remain slim, with their namespace still denoted by the object they are members of, but avoids any value collisions.