Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3: static query dependency tree #8

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Conversation

ahdinosaur
Copy link
Member

had a radical idea for a new way to do queries, which eliminates the stupid query() + shouldQueryAgain() madness.

@iainkirkpatrick
Copy link
Member

on it 🏀

@ahdinosaur
Copy link
Member Author

picking this back up

@ahdinosaur
Copy link
Member Author

published 3.0.0-pre.0 as prerelease

(with npm publish --tag prerelease so as not to publish as latest when you npm install feathers-action-react)

@ahdinosaur
Copy link
Member Author

upgraded:

have yet to do anything complex, like dependent queries

i *think* there was a bug where the two states were not being set at the
same time, which led to a situation where the args had changed but the
cid was not yet set, causing an infinite loop.
@ahdinosaur ahdinosaur changed the title [wip] v3: static query dependency tree v3: static query dependency tree Feb 15, 2018
@ahdinosaur
Copy link
Member Author

ready for review @iainkirkpatrick @gregorykan!

@ahdinosaur
Copy link
Member Author

notice a bug when navigating to the same container with different route params, the container thinks all the queries have already happened, so doesn't re-query. need a way to invalidate the entire query set, like match: selector so match: getCurrentThingId.

@ahdinosaur
Copy link
Member Author

open question: should a query's selectors (id and/or params) be resolved before the dependencies are ready?

example:

      {
        name: 'childTaskWorks',
        service: 'taskWorks',
        dependencies: [
          'childTaskPlans',
        ],
        params: createSelector(
          getCurrentTaskPlan,
          pipe(
            prop('childTaskPlans'),
            map(prop('id')),
            (childTaskPlanIds) => ({
              query: {
                taskPlanId: {
                  $in: childTaskPlanIds
                }
              }
            })
          )
        )
      }

on the one hand, selectors should probably be defensive anyways, so they should work no matter the data. on the other hand, we could handle this case to be helpful and friendly.

@ahdinosaur
Copy link
Member Author

patch to fix the dependent query selectors:

diff --git a/index.js b/index.js
index 2349cf9..ab0d871 100644
--- a/index.js
+++ b/index.js
@@ -270,19 +270,23 @@ const getQuerys = createSelector(
       const hasReadyDependencies = hasReadyDependenciesByQuery[query.name]
 
       const isStarted = not(isNil(cid))
-      // TODO clean up
-      const id = resolveQueryArg('id')(null)(query)(state, ownProps)
-      // need to clone params because feathers mutates the params during a request.
-      const params = clone(resolveQueryArg('params')({})(query)(state, ownProps))
-      var prevArgs
-      if (isStarted) {
-        prevArgs = {
+      var prevArgs = isStarted
+        ? {
           // TODO clean up
           id: isNil(args.id) ? null: args.id,
           params: isNil(args.params) ? {}: args.params
         }
-      } else {
-        prevArgs = null
+        : {
+          id: null,
+          params: null
+        }
+      var id, params
+      // don't resolve id or params until dependencies are ready
+      if (hasReadyDependencies) {
+        // TODO clean up
+        id = resolveQueryArg('id')(null)(query)(state, ownProps)
+        // need to clone params because feathers mutates the params during a request.
+        params = clone(resolveQueryArg('params')({})(query)(state, ownProps))
       }
       const nextArgs = { id, params }
       const isSameArgs = deepEqual(prevArgs, nextArgs)

@ahdinosaur
Copy link
Member Author

and how to make selector more defensive:

      {
        name: 'childTaskWorks',
        service: 'taskWorks',
        dependencies: [
          'childTaskPlans',
        ],
        params: createSelector(
          getCurrentTaskPlan,
          ifElse(
            isNil,
            () => null,
            pipe(
              prop('childTaskPlans'),
              map(prop('id')),
              (childTaskPlanIds) => ({
                query: {
                  taskPlanId: {
                    $in: childTaskPlanIds
                  }
                }
              })
            )
          )
        )
      }

@ahdinosaur
Copy link
Member Author

one idea: what if you could depend on a query in a parent component? we could register each query by name in the global react context, rather than local react state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants