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

Multi-argument custom query invalidation doesn't seem to work #915

Open
strblr opened this issue Apr 22, 2022 · 6 comments
Open

Multi-argument custom query invalidation doesn't seem to work #915

strblr opened this issue Apr 22, 2022 · 6 comments

Comments

@strblr
Copy link

strblr commented Apr 22, 2022

In a project, I have lists of entities called "hypotheses" that need to be refreshed via live queries. These lists are fetched using the following query:

type Query {
  hypotheses(
    project: ID!
    law: Int
    filters: ListFiltersInput!
  ): HypothesisPage!
}

Hypotheses are tied to specific "laws" (identified by integers). The second argument law can be null which returns all hypotheses regardless of their laws.
I'm trying to invalidate based on a custom index that looks like this:

{
  field: "Query.hypotheses",
  args: ["project", "law"]
}

Here is the document I use on the frontend:

export const HypothesesQuery = gql`
  ${HypothesisPageFragment}

  query Hypotheses($project: ID!, $law: Int, $filters: ListFiltersInput!)
    @live {
    hypotheses(project: $project, law: $law, filters: $filters) {
      ...HypothesisPage
    }
  }
`;

Now when creating an hypothesis with a createHypothesis mutation (resolver is successfully called), the following invalidation doesn't work:

invalidate([
  `Query.hypotheses(project:"${project._id.toHexString()}",law:null)`,
  `Query.hypotheses(project:"${project._id.toHexString()}",law:${
    hypothesis.law
  })`
]);

(project._id.toHexString() has the correct value)
Any idea of what's going on? Thank you.

@strblr
Copy link
Author

strblr commented Apr 22, 2022

Ok by logging the generated indices, I can see what's going on. Here are some:

  'Query.hypotheses(project:"61008f309a516c36b4505363",law:"2")',
  'Query.hypotheses(project:"61008f309a516c36b4505363",law:"null")',
  'Query.hypotheses(project:"61008f309a516c36b4505363",law:"3")'

Non-string values seem to be cast to strings before generating the keys.

Edit: Issue seems to come from here:

if (indicesForCoordinate) {
for (const index of indicesForCoordinate) {
let parts: Array<string> = [];
for (const part of index) {
if (Array.isArray(part)) {
if (args[part[0]] === part[1]) {
parts.push(`${part[0]}:"${args[part[0]]}"`);
}
} else if (args[part] !== undefined) {
parts.push(`${part}:"${args[part]}"`);
}
}
if (parts.length) {
liveQueyContext.addResourceIdentifier(
`${fieldCoordinate}(${parts.join(",")})`
);
}
}
}

This doesn't seem right.

@n1ru4l
Copy link
Owner

n1ru4l commented May 12, 2022

What behavior would you consider instead? If the type would implement a .toString() function you can customize how the thing is stringified 🤔

@strblr
Copy link
Author

strblr commented May 12, 2022

@n1ru4l Well since the base scalars are JSON-ish, you could use JSON.stringify for numbers, strings, null and booleans, and .toString() for other values. This would make the indices closer to actual GraphQL syntax. What do you think?

@n1ru4l
Copy link
Owner

n1ru4l commented May 16, 2022

Why couldn't your id type implement a toString method?

class ID extends YourId {
  toString() {
    return this.toHexString()
  }
}

Relying on JSON.stringify is not safe and would require using something as https://www.npmjs.com/package/json-stable-stringify

I do not recommend using complex types for indices. I think it is safe to say that indices should only be set on values that can easily be serialized to a string.

@strblr
Copy link
Author

strblr commented May 16, 2022

You would still have the hard-coded double quotes, which can bring weird indices collision. For example imagine a nullable query argument of type String: null and "null" would serialize to the same index.

@n1ru4l
Copy link
Owner

n1ru4l commented May 16, 2022

I see your point, in that case, JSON.stringify (aka https://www.npmjs.com/package/json-stable-stringify) might be the best solution. 🤔

I think adding json-stable-stringify as a dependency should be fine.

@strblr Would you like to create a PR with some tests and an implementation?

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

No branches or pull requests

2 participants