devTools prop
Ashoat opened this issue · 5 comments
Motivation
Many dev tools and debugging libraries try to track "actions" or "events" that occur in an app.
In my case, in production I track events so that I can report them in the case of a crash. This makes it easier to understand what led to a crash, and oftentimes I can glean repro steps just by looking at the log.
In dev mode, my app reports these events directly to a debugging console. Reactotron is a good option for this, especially now that it can integrated into Flipper. Redux Dev Tools can also be used for this sort of tracking functionality, similar to how it is done in ReactNav today.
Implementation
Before ReactNav5 it was possible to rely on the onNavigationStateChange
prop for this. But in ReactNav5, the onStateChange
prop only reports the new navigation state, without any identifying action. This makes it hard to understand an event at-a-glance.
Because state changes in ReactNav5 don't necessarily derive from actions, matching them back up to actions is something that needs to be done post-hoc. The logic for doing this for the existing Redux Dev Tools integration currently resides in the useDevTools
hook.
I'd like to extract that functionality to a public API so users don't need to reimplement it.
API
We'll probably want to expose a prop on NavigationContainer
.
It's important to avoid giving the user the wrong ideas about this API. Because of the prevalence of the Redux action/reducer/state model, it's easy for the user to assume that all state changes are a function of the actions. If they take this assumption on faith, and implement business logic or persistence (for instance) based on that assumption, they will have a bad time.
For that reason it's probably best to name this prop something like devTools
. We could allow the user to pass in an object that contains various callbacks that ReactNav could trigger:
trackAction
- triggered whenever an action is dispatched. Passed the action object and the new state†trackStateChange
- triggered whenever the state changes. Passed the new state, along with a list of actions that have occurred since the last state change. This list may be empty, for instance in the case of hydrating a stale navigation state, or in the case of the user dynamically changing route definitions
I'm not tied to this particular API - open to any comments & suggestions!
† Right now, if the user calls resetRoot
on the NavigationContainer
ref, this passes a string action to useDevTools
(@@RESET_ROOT
). This feels fairly nonstandard, and would probably be best to avoid in an external API. If we want to keep this we could add an onResetRoot
callback to the devTools
prop
Hey Ashoat! I really like the idea of cleaning up useDevTools
to use a public API rather than the internally-hardcoded global.__REDUX_DEVTOOLS_EXTENSION__
.
Good point that state changes do not map 1-1 to actions. But, I think people might want to track actions for other reasons than devTools, so maybe we should just add an onAction
prop to the container?
My other concern is that devTools.trackStateChange
seems very similar to the existing onStateChange
prop.
We could possibly provide a default behavior to onStateChange
and onAction
which calls out to the redux devtools. There are other ways to sugar-coat this, including the devTools prop you described.
I think you're on the right track, I just hope to keep the container props as simple as possible.
That makes sense. I'm curious to hear @satya164's thoughts, as he initially suggested explicitly mentioning "dev tools" in the naming to avoid any confusion.
I'm pretty flexible on the API, mostly just want a way for users of the library to track actions.
onAction
would be very useful, I was actually trying to implement preloading requests with relay (starting the request as soon as the navigation action is dispatched instead of waiting for the screen to render) but it was pretty hard and required a custom router.
I think people might want to track actions for other reasons than devTools
Actually the reason I talked to @Ashoat about a devtools specific API is because I don't want people to be tracking actions other than for debugging purposes. It can cause subtle issues and coupling when people start to track actions imo.
For example:
- Action names don't matter, a
reset
action could do the same thing as anavigate
or apush
, what matters is what changed, not the action that caused the change. - If people start relying on actions, it makes it impossible to change our navigator code. For example, if we changed
goBack()
topop()
, it will break user code if someone is relying on listening to it to detect going back. - Since navigation configuration is dynamic, other things can cause the state to change (such as user's component state), it's not always 1-1 for action -> state. People relying on actions to detect changes will subtle bugs.
Instead, people should rely on checking what changed in the state, e.g. - the focused screen changed, so change statusbar style or something. I think this also works like React works, run an effect based on what changed, not how it changed.
onAction
would be very useful, I was actually trying to implement preloading requests with relay (starting the request as soon as the navigation action is dispatched instead of waiting for the screen to render) but it was pretty hard and required a custom router.
Isn't action -> screen rendering almost instant? At least for network requests, a few milliseconds doesn't seem like it would be noticeable.
Anyway, the use case seems to be to get notified before screen render, not necessarily get notified of actions. An action might not even change anything. With suspense, I assume we'd need some sort of API for preloading anyway, so it's something we'd need to think about.
Another case to know before state change is to be able to prevent a state change, e.g. prevent going back from the screen (e.g. if user is editing something), but it also doesn't need to be an action listener (rather can't be, goBack
and pop
should both work for this for example).
For this use case, there would need to be some API that'd need to notify with upcoming state before it's applied, with ability to prevent it, or some more specific APIs for specific purposes.
If separate devtools API isn't something we want, to avoid duplication, another option is to expose something like containerRef.addListener('__UNSAFE__action', callback)
to emphasize that this is unsafe.
Then the devtools stuff can be implemented in a hook and used like:
useReduxDevToolsExtension(containerRef)
The container's ref
has other methods like getRootState
, resetRoot
, dispatch
etc. for the devtools to interact with the navigator's state.