facebook/relay

@live LiveState unsubscribe cleanup function never gets called

Closed this issue · 6 comments

/**
 * @RelayResolver Query.liveSubUnsubTest: Int
 * @live
 */
export function liveSubUnsubTest(): LiveState<number> {
  return {
    read() {
      console.log("liveSubUnsubTest", "read")
      return Date.now()
    },
    subscribe(didChange) {
      console.log("liveSubUnsubTest", "subscribe")
      const id = setInterval(() => {
        console.log("liveSubUnsubTest", "tick")
        didChange()
      }, 1234)
      return function unsubscribe() {
        console.log("liveSubUnsubTest", "unsubscribe")
        clearInterval(id)
      }
    },
  }
}
function LiveSubUnsubTestDemo() {
  const data = useClientQuery<LiveState__LiveSubUnsubTestDemo__Query>(
    graphql`
      query LiveState__LiveSubUnsubTestDemo__Query {
        liveSubUnsubTest
      }
    `,
    {},
  )

  if (!data.liveSubUnsubTest) {
    return <Text className="text-white">MIA</Text>
  }

  return (
    <>
      <Text className="text-white">
        {new Date(data.liveSubUnsubTest).toLocaleString()}
      </Text>
    </>
  )
}

after unmounting LiveSubUnsubTestDemo, I'm still getting the subscription ticks forever

 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick

console.log("liveSubUnsubTest", "unsubscribe") is never called

Tried forcing root queries to expire ASAP with queryCacheExpirationTime: 1.

  const source = new RecordSource()
  const store = new RelayStore(source, {
    queryCacheExpirationTime: 1,
  })

It successfully causes __gc() to log zero marked references (indicating that nothing is being retained).

But the LiveState unsubscribe is still never called.

 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick
 LOG  liveSubUnsubTest tick

If references.size === 0, the entire _recordSource is cleared using _recordSource.clear().

This means that no records, including those with resolver subscriptions, will exist in the store. Consequently, the loop that iterates over storeIDs and checks for maybeResolverSubscription will never execute, and the subscriptions won't be disposed of properly.

BOOM! Solved it.

If I comment out the references.size === 0 if branch, my LiveState is properly unsubscribed as expected.

      // if (references.size === 0) {
      //   console.debug('(RELAY)', 'RelayModernStore._collect clearing all records - no references');
      //   this._recordSource.clear();
      // } else {
...
      // }

PROBLEM: This code bypasses maybeResolverSubscription

// Sweep records without references
if (references.size === 0) {
// Short-circuit if *nothing* is referenced
this._recordSource.clear();
} else {

seems like we need to call maybeResolverSubscription sometimes

const maybeResolverSubscription = RelayModernRecord.getValue(
record,
RELAY_RESOLVER_LIVE_STATE_SUBSCRIPTION_KEY,
);
if (maybeResolverSubscription != null) {
// $FlowFixMe - this value if it is not null, it is a function
maybeResolverSubscription();
}

quick and dirty workaround

  const source = new RecordSource()
  const store = new RelayStore(source, {
    // WARNING: DO NOT REMOVE THIS LINE
    // This is a workaround that unbreaks garbage collection in the Relay store
    // See https://github.com/facebook/relay/issues/4830 for more info
    // Without this, root queries will never be disposed and LiveState values will never unsubscribe
    queryCacheExpirationTime: 1,
  })
  const network = Network.create(fetchOrSubscribe, fetchOrSubscribe)
  const env = new Environment({ store, network })

  // WARNING: DO NOT REMOVE THIS LINE
  // This is a workaround that unbreaks garbage collection in the Relay store
  // See https://github.com/facebook/relay/issues/4830 for more info
  // RelayModernStore has two branches for clearing the store, one of which is broken
  // this line forces the store to use the working branch
  // by forcing the store to retain an extra root query
  // because the failing branch checks `if (references.size === 0)`
  env.retain(createOperationDescriptor(getRequest(graphql`query theRelayEnvironment__DummyQuery($id: ID!) { node(id:$id) { id } }`), { id: ROOT_ID })) // prettier-ignore

related docs: #4831