Fix fragmentToResolve + Scheduler improvements

samdenty · samdenty · commit 9fe366b6695b · 2019-11-07T17:59:41.000Z
diff --git a/.changeset/itchy-trees-refuse.md b/.changeset/itchy-trees-refuse.md
@@ -8,18 +8,19 @@
 '@gqless/utils': patch
 ---
 
-## Cache
+**Cache**
 
 - Support for Keys
-  - Support for circular references
+  - They can now be used circularly
 - Support for partial merging
 
-## Optimizations
+**Optimizations**
 
 - Heavily reduced cost of merging to cache
 - Extension instances are now shared
 - Internal lazy intialized & memoized changes
 
-## Regressions
+**Fixes**
 
-- Fragments need to be rewired, currently don't work
+- Scheduler is now smarter, works reliably with concurrent mode
+- Fragments containing selections with arguments now work
diff --git a/SPEC/TODO.md b/SPEC/TODO.md
@@ -5,9 +5,8 @@
   - Errors can make non-nullable fields return null
 
 - Make `gqless` bin alias to `@gqless/cli`, so only need to install 1 set of deps
-- Make codegen default to typescript, add --js option
 - Make variable updates re-fetch & update component
-- Default key logic should instead be moved to codegen. More explicit and no magic happening
+- Default key logic should be moved to codegen. More explicit and no magic happening
 
 - When we get an array
 
@@ -20,6 +19,7 @@
 # Improvements
 
 - Warn when \_\_typename is accessed for React, without an ofType call
+  - Support it as well
 - Make toTree output deterministicly
 - Prevent duplication in buildSelections
 - Make Object.set default to `match` instead of setData
diff --git a/examples/react-github/src/components/Repo.tsx b/examples/react-github/src/components/Repo.tsx
@@ -4,9 +4,15 @@ import { graphql } from '@gqless/react'
 
 export const Repo = graphql(
   ({ repo }: { repo: Types.Repository }) => {
+    // if (ofType(repo.owner, 'Organization')) {
+    //   console.log(repo.owner.organizationBillingEmail)
+    // }
+
     return (
       <div>
-        <h3>{repo.name}</h3>
+        <h3>
+          '{repo.owner.__typename}' {repo.name}
+        </h3>
         {repo.descriptionHTML && (
           <p dangerouslySetInnerHTML={{ __html: repo.descriptionHTML }} />
         )}
diff --git a/examples/react-github/src/components/User.tsx b/examples/react-github/src/components/User.tsx
@@ -3,8 +3,6 @@ import * as Types from '../graphql'
 import { graphql, useFragment } from '@gqless/react'
 import { Repo } from './Repo'
 import styled from 'styled-components'
-import { query } from '../graphql'
-import { getAccessor } from 'gqless'
 
 const StyledUser = styled.div`
   display: flex;
@@ -18,8 +16,7 @@ const Repos = styled.div`
 
 export const User = graphql(
   ({ user }: { user: Types.User }) => {
-    let userFragment = user
-    userFragment = useFragment(user)
+    const userFragment = useFragment(user)
 
     const repos = userFragment.repositories({
       first: 100,
diff --git a/examples/react-github/src/index.tsx b/examples/react-github/src/index.tsx
@@ -2,11 +2,14 @@ import React from 'react'
 import ReactDOM from 'react-dom'
 import { App } from './App'
 
-ReactDOM.render(
+ReactDOM.unstable_createRoot(document.getElementById('app')).render(
   <React.Suspense fallback="loading app">
     <App />
-  </React.Suspense>,
-  document.getElementById('app')
+  </React.Suspense>
 )
-
-Object.assign(window, {})
+// ReactDOM.render(
+//   <React.Suspense fallback="loading app">
+//     <App />
+//   </React.Suspense>,
+//   document.getElementById('app')
+// )
diff --git a/gqless/src/Accessor/Accessor.ts b/gqless/src/Accessor/Accessor.ts
@@ -97,21 +97,17 @@ export abstract class Accessor<
   }
 
   public get data() {
-    try {
-      if (this._data === undefined) {
-        try {
-          this.data = this.getData()
-        } catch (accessor) {
-          if (accessor instanceof Accessor) return accessor.data
-
-          throw accessor
-        }
-      }
+    if (this.fragmentToResolve) {
+      return this.fragmentToResolve.data
+    }
 
-      return this._data
-    } finally {
-      accessorInterceptors.forEach((intercept) => intercept(this))
+    if (this._data === undefined) {
+      this.data = this.getData()
     }
+
+    accessorInterceptors.forEach((intercept) => intercept(this))
+
+    return this._data
   }
   public set data(data: any) {
     this._data = data
@@ -295,11 +291,3 @@ export abstract class Accessor<
     }
   }
 }
-
-export const getAccessorData = (accessor: Accessor): any => {
-  if (accessor.fragmentToResolve) {
-    return getAccessorData(accessor.fragmentToResolve)
-  }
-
-  return accessor.data
-}
diff --git a/gqless/src/Node/ArrayNode.ts b/gqless/src/Node/ArrayNode.ts
@@ -1,7 +1,7 @@
 import { Generic, Mix } from 'mix-classes'
 
 import { IndexAccessor } from '../Accessor'
-import { ACCESSOR, getAccessorData } from './../Accessor'
+import { ACCESSOR } from '../Accessor'
 import {
   NodeContainer,
   Matchable,
@@ -93,7 +93,7 @@ export class ArrayNode<TNode> extends Mix(
                 ctx.accessor.get(index) ||
                 new IndexAccessor(ctx.accessor, index)
 
-              return getAccessorData(accessor)
+              return accessor.data
             }
 
             return (this.ofNode as any as DataTrait).getData({
diff --git a/gqless/src/Node/InterfaceNode.ts b/gqless/src/Node/InterfaceNode.ts
@@ -47,10 +47,8 @@ export class InterfaceNode<TImplementation>
 
     return new Proxy(data, {
       get: (_, prop: any) => {
-        // if (accessor.fragmentToResolve) {
-        //   const { data } = accessor.fragmentToResolve
-        //   return data ? data[prop] : undefined
-        // }
+        const fragment = ctx.accessor?.fragmentToResolve
+        if (fragment) return fragment.data?.[prop]
 
         // If the prop exists in this interface,
         // return directly from interface
@@ -78,11 +76,12 @@ export class InterfaceNode<TImplementation>
       },
 
       set: (_, prop: string, value) => {
-        // if (accessor.fragmentToResolve) {
-        //   const { data } = accessor.fragmentToResolve
-        //   if (data) data[prop] = value
-        //   return true
-        // }
+        const fragment = ctx.accessor?.fragmentToResolve
+        if (fragment) {
+          const { data } = fragment
+          if (data) data[prop] = value
+          return true
+        }
 
         if (prop === '__typename') return true
 
diff --git a/gqless/src/Node/ObjectNode.ts b/gqless/src/Node/ObjectNode.ts
@@ -63,10 +63,8 @@ export class ObjectNode extends Mix(
 
     return new Proxy({} as any, {
       get: (_, prop: any) => {
-        // if (accessor.fragmentToResolve) {
-        //   const { data } = accessor.fragmentToResolve
-        //   return data ? data[prop] : undefined
-        // }
+        const fragment = ctx.accessor?.fragmentToResolve
+        if (fragment) return fragment.data?.[prop]
 
         if (prop === ACCESSOR) return ctx.accessor
         // Statically resolve __typename
@@ -89,11 +87,12 @@ export class ObjectNode extends Mix(
       },
 
       set: (_, prop: string, value) => {
-        // if (accessor.fragmentToResolve) {
-        //   const { data } = accessor.fragmentToResolve
-        //   if (data) data[prop] = value
-        //   return true
-        // }
+        const fragment = ctx.accessor?.fragmentToResolve
+        if (fragment) {
+          const { data } = fragment
+          if (data) data[prop] = value
+          return true
+        }
 
         if (prop === '__typename') return true
 
diff --git a/gqless/src/Node/abstract/Abstract.ts b/gqless/src/Node/abstract/Abstract.ts
@@ -50,10 +50,8 @@ export class Abstract<TNode extends ObjectNode = ObjectNode>
       {},
       {
         get(_, prop: any) {
-          // if (accessor.fragmentToResolve) {
-          //   const { data } = accessor.fragmentToResolve
-          //   return data ? data[prop] : undefined
-          // }
+          const fragment = ctx.accessor?.fragmentToResolve
+          if (fragment) return fragment.data?.[prop]
 
           if (prop === ACCESSOR) return ctx.accessor
 
@@ -71,11 +69,12 @@ export class Abstract<TNode extends ObjectNode = ObjectNode>
         },
 
         set(_, prop: any, value: any) {
-          // if (accessor.fragmentToResolve) {
-          //   const { data } = accessor.fragmentToResolve
-          //   if (data) data[prop] = value
-          //   return true
-          // }
+          const fragment = ctx.accessor?.fragmentToResolve
+          if (fragment) {
+            const { data } = fragment
+            if (data) data[prop] = value
+            return true
+          }
 
           // else set it on the first extension with the property
           for (const extension of getExtensions(ctx)) {
diff --git a/gqless/src/Node/abstract/FieldsNode/FieldNode.ts b/gqless/src/Node/abstract/FieldsNode/FieldNode.ts
@@ -1,7 +1,7 @@
 import { invariant } from '@gqless/utils'
 import { deepJSONEqual, computed } from '../../../utils'
 import { Arguments } from '../../Arguments'
-import { FieldAccessor, getAccessorData } from '../../../Accessor'
+import { FieldAccessor } from '../../../Accessor'
 import { FieldSelection } from '../../../Selection'
 import { EnumNode } from '../../EnumNode'
 import { ScalarNode } from '../../ScalarNode'
@@ -65,7 +65,7 @@ export class FieldNode<TNode extends DataTrait  = DataTrait> extends NodeContain
           ctx.accessor.get(selection) ||
           new FieldAccessor(ctx.accessor, selection)
 
-        return getAccessorData(accessor)
+        return accessor.data
       }
 
       return this.ofNode.getData({
diff --git a/gqless/src/Scheduler/Commit.ts b/gqless/src/Scheduler/Commit.ts
@@ -8,6 +8,9 @@ import { createEvent } from '@gqless/utils'
 const defaultQuery = new Query()
 
 export class Commit extends Disposable {
+  public onActive = createEvent()
+  public onIdle = createEvent()
+
   public onFetched = createEvent()
   public accessors = new Map<Accessor, Query[]>()
 
@@ -45,19 +48,21 @@ export class Commit extends Disposable {
     // or being (re-)fetched from a previous commit, don't re-fetch it
     if (this.disposed || accessor.status !== NetworkStatus.idle) return unstage
 
-    // If we already have the parent, remove the
-    // parent to narrow down the accessors. This is used when a accessor is created
-    // this could cause issues later, may need to add a recurse field to handle polling etc.
-    if (accessor.parent && this.accessors.has(accessor.parent)) {
-      this.unstage(accessor.parent)
-    }
+    if (!this.accessors.size) this.onActive.emit()
 
     accessor.status = accessor.value
       ? NetworkStatus.updating
       : NetworkStatus.loading
 
     this.accessors.set(accessor, [...this.stack, ...queries])
 
+    // If we already have the parent, remove the
+    // parent to narrow down the accessors. This is used when a accessor is created
+    // this could cause issues later, may need to add a recurse field to handle polling etc.
+    if (accessor.parent && this.accessors.has(accessor.parent)) {
+      this.unstage(accessor.parent)
+    }
+
     return unstage
   }
 
@@ -71,6 +76,8 @@ export class Commit extends Disposable {
     }
 
     this.accessors.delete(accessor)
+
+    if (!this.accessors.size) this.onIdle.emit()
   }
 
   public async fetch() {
diff --git a/gqless/src/Scheduler/Scheduler.ts b/gqless/src/Scheduler/Scheduler.ts
@@ -17,12 +17,12 @@ export class Scheduler extends Disposable {
   constructor(
     private fetchAccessors: AccessorFetcher,
     public plugins: Plugins = new Plugins(),
-    public interval = 1000
+    public interval = 50
   ) {
     super()
 
-    this.start()
-    this.addDisposer(this.cancel)
+    this.startTimer()
+    this.addDisposer(this.clearTimer)
   }
 
   public pushStack(...queries: Query[]) {
@@ -45,8 +45,8 @@ export class Scheduler extends Disposable {
     }
   }
 
-  private start() {
-    this.cancel()
+  private startTimer() {
+    this.clearTimer()
 
     // Don't create new Commit, if prev one unused
     if (!this.commit || this.commit.accessors.size) {
@@ -55,13 +55,23 @@ export class Scheduler extends Disposable {
       this.commit = new Commit(this.plugins, this.stack, this.fetchAccessors)
     }
 
-    this.timer = setTimeout(() => {
-      this.commit.fetch()
-      this.start()
-    }, this.interval)
+    const { commit } = this
+    commit.onActive.then(() => {
+      this.timer = setTimeout(() => {
+        commit.fetch()
+        this.startTimer()
+      }, this.interval)
+
+      commit.onIdle.then(() => {
+        if (commit !== this.commit) return
+
+        // Cancel timer, and wait until commit is active again
+        this.startTimer()
+      })
+    })
   }
 
-  private cancel() {
+  private clearTimer() {
     clearTimeout(this.timer)
   }
 }
diff --git a/packages/react/src/graphql/graphql.tsx b/packages/react/src/graphql/graphql.tsx
diff --git a/packages/react/src/ofType/ofType.ts b/packages/react/src/ofType/ofType.ts
