Regression on ID scalar type filtering

[HiiiiD]

Describe the regression
Up until version 4.0, there was a bug in which String and IDs were using the same features.filters.
In this PR the issue was fixed but the Neo4jIDFiltersSettings wasn't changed, so it wasn't possible anymore to use _GT,_GTE,_LT,_LTE on the ID type

Last working version

Worked up to version: @neo4j/graphql@4.0.0

Stopped working in version: @neo4j/graphql@4.0.0

Type definitions

To Reproduce

Expected behavior
I expected to see the possibility of adding the other scalar filters

Screenshots

System (please complete the following information):

• OS: Windows
• Node.js version: 20.12.0

Additional context

[Liam-Doodson]

Hi @HiiiiD! I believe these filters were removed intentionally. As IDs serialise to strings these filters don't really make sense as it is not immediately clear what LT/GT is in relation to (e.g. is it comparing length of string/alphabetically?).

What is the use case you are trying to solve here?

[HiiiiD]

@Liam-Doodson Given that the ID type is mostly used as a unique identifier for an entity, I mostly use it for pagination, instead of using skip/limit.
For example if you have a type like this:

type User {
 createdAt: DateTime!
 id: ID! @id @unique

In case you have two User with the same value for createdAt, you can leverage string sorting on the ID to allow deterministic behaviour.
What do you think?

[Liam-Doodson]

It sounds like what you really want is cursor-based pagination then? We currently enable this on connection fields (e.g. userConnection.

We are currently considering adding this to more places (although this isn't likely to be until a future major release)

[angrykoala]

In addition to @Liam-Doodson comment.

Deterministic cursor pagination is something that we are planning on adding in future versions of the library, in the meantime, your approach seems to be correct, but I would recommend you define it as a String field in Graphql instead of number. As mentioned, this use case is really cursor based pagination and the filters for IDs don't really make much sense out of that scope.

If you don't need strictly deterministic cursor pagination, I would recommend cursor based pagination over the id field with the connection endpoint instead as Liam mentioned

[HiiiiD]

Yep, due to the fact that's only available on the connection and not on the main OGM Model, I had to implement it this way.
Also, given that they're serialized as strings, why don't we have the same methods? What's the downside?

[Liam-Doodson]

Yeah honestly, I think it's more a mistake that we left these filters as they are for strings as it's not really clear what they are comparing (even if they are hidden behind the features config when constructing the library).

The problem with adding stuff like this is that we don't want to support features that don't really make sense. As this library has grown in complexity with features like this being added there is now a lot of edge cases that need to be considered, tested and ideally documented with every new feature that is added. For example, what happens if these filters are used within relationship filters (e.g. where: {movies_SINGLE: {id_GT: "foo"}})? Does our translation work in this case? Is there some weird combination of filters that breaks this etc?

Adding complexity like this makes it harder and harder to actually add the things which are going to bring the most value to users (e.g. in this case probably proper cursor-based pagination on the base fields). I hope that makes sense!

[HiiiiD]

Okk, makes sense then. I'll stick to the previous version until the curson pagination will be implemented on the base fields.
Thanks

P.S: Should I close the issue?

[Liam-Doodson]

Yeah I think that's probably the best option for now so I'll close this issue. For reference this is the issue we have for cursor-based pagination on base fields #1157 and this is the issue we have for the deterministic cursor-based pagination #2685