Implement ContextUtils as mixins #3440
Conversation
| const contextName = makeContextName(name) | ||
| const listenersKey = `${contextName}/listeners` |
There was a problem hiding this comment.
Keep these folks scoped to avoid any possible name clashes
There was a problem hiding this comment.
Why would there be name clashes? The context key in this case is going to be @@contextSubscriber/router – I fixed it from the original implementation where I munged the original context object unnecessarily.
There was a problem hiding this comment.
If you define something like mixins: [ContextSubscriber('router'), ContextSubscriber('whatever')], React would complain that they both define subscribe method. Even worse, if that subscribe stops being a method, React will blindly merge mixins and thus they would each overwrite the same this.listeners.
Finally, the consuming component (such as <Router>) may add an instance field called this.listeners or a method called subscribe. By scoping mixin method and field names by the corresponding context name, we are playing it safe. We also signal both internally and externally this way that those things (e.g. this.subscribe) are not supposed to be called directly even though technically they are available on the <Router> instance.
As discussed in #3439, these are implementation details and are better implemented as mixins so that it’s easier to kill them later. They don’t leak into public API, and React mixin merging behavior comes in handy here.
This way the hierarchy stays flat but we work around facebook/react#2517.