Skip to content
New issue

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

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

Already on GitHub? # 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?

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

No branches or pull requests

2 participants