Add flag to skip circular dependency detection (#2214)

Summary:
Pull Request resolved: #2214

I discovered a bug in `detectCircularDependencies()` where it may erroneously detect a circular dependency when `graphQLQueryEffect()` is used. It arises from the fact that we are using a global stack to keep track of which Atom/Selectors have been *visited*.

From the stack trace, I think I've figured out what is happening...
1) There are a bunch of `selectorGet()` calls to load dependencies.
2) This calls `detectCircularDependencies()` several times and a bunch of keys get added to the `dependencyStack`, as you would expect. So far, everything looks good.
3) Then, because it is a nested dependency, we load an Atom that has a `graphQLQueryEffect()`.
4) The `graphQLQueryEffect()` subscribes to a `RelayObservable`.The act of subscribing schedules a task using the `RelayFBScheduler`.
5) The schedule invokes `flushSyncCallbacks()` in ReactDOM.
6) Which calls `renderWithHooks()` in ReactDOM.
7) Which renders a component that calls `useRecoilValue()`.
8) We then end up back in `detectCircularDependencies()`, which is where we fail because the `dependencyStack` already includes the keys of all those previous Selectors and Atoms up to and including the Atom with the `graphQLQueryEffect()`.

This change adds a flag prop named `skipCircularDependencyDetection_DANGEROUS` to `<RecoilRoot>`, which is propagated down to the `Store`. If this flag is set to `true` then `detectCircularDependencies()` is bypassed.

This is obviously *less than ideal*. I think we can all agree that a fix for `detectCircularDependencies()` would be a much better solution. I am under the impression that, that will be a major refactor though, since it will require changing `dependencyStack` to be non-global and passing it all around. This flag will at least mitigate the bug until a more proper solution can be implemented. It should be very easy to back it out once that happens.

Differential Revision: D44563103

fbshipit-source-id: b396ad068b5dc511664d204e326086cc6b8af29c

Author: Ryan Holdren

packages/recoil/core/Recoil_RecoilRoot.js

@@ -65,6 +65,7 @@ type InternalProps = {
   initializeState?: MutableSnapshot => void,
   store_INTERNAL?: Store,
   children: React.Node,
+  skipCircularDependencyDetection_DANGEROUS?: boolean,
 };
 
 function notInAContext() {
@@ -365,6 +366,7 @@ function RecoilRoot_INTERNAL({
   initializeState,
   store_INTERNAL: storeProp, // For use with React "context bridging"
   children,
+  skipCircularDependencyDetection_DANGEROUS,
 }: InternalProps): React.Node {
   // prettier-ignore
   // @fb-only: useEffect(() => {
@@ -486,6 +488,7 @@ function RecoilRoot_INTERNAL({
         getGraph,
         subscribeToTransactions,
         addTransactionMetadata,
+        skipCircularDependencyDetection_DANGEROUS,
       },
   );
   if (storeProp != null) {
@@ -550,6 +553,7 @@ type Props =
       store_INTERNAL?: Store,
       override?: true,
       children: React.Node,
+      skipCircularDependencyDetection_DANGEROUS?: boolean,
     }
   | {
       store_INTERNAL?: Store,
@@ -562,6 +566,7 @@ type Props =
        */
       override: false,
       children: React.Node,
+      skipCircularDependencyDetection_DANGEROUS?: boolean,
     };
 
 function RecoilRoot(props: Props): React.Node {

packages/recoil/core/Recoil_State.js

@@ -132,6 +132,7 @@ export type Store = $ReadOnly<{
   getGraph: StateID => Graph,
   subscribeToTransactions: ((Store) => void, ?NodeKey) => {release: () => void},
   addTransactionMetadata: ({...}) => void,
+  skipCircularDependencyDetection_DANGEROUS?: boolean,
 }>;
 
 export type StoreRef = {

packages/recoil/recoil_values/Recoil_selector.js

@@ -1102,6 +1102,9 @@ function selector<T>(
   }
 
   function selectorGet(store: Store, state: TreeState): Loadable<T> {
+    if (store.skipCircularDependencyDetection_DANGEROUS === true) {
+      return getSelectorLoadableAndUpdateDeps(store, state);
+    }
     return detectCircularDependencies(() =>
       getSelectorLoadableAndUpdateDeps(store, state),
     );