feat: typescript utilities

[tien]

• Improve structures type definitions

type Person = Vertex<"person", { name: string; age: number }>;
type Pet = Vertex<"pet", { name: string; age: number }>;
type Owns = Edge<Person, "owns", Pet, { since: Date }>;

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.77%. Comparing base (2d32517) to head (459bee2).
Report is 135 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2599      +/-   ##
============================================
+ Coverage     76.16%   76.77%   +0.61%     
- Complexity    13170    13180      +10     
============================================
  Files          1085     1088       +3     
  Lines         65189    66322    +1133     
  Branches       7289     7304      +15     
============================================
+ Hits          49651    50922    +1271     
+ Misses        12830    12685     -145     
- Partials       2708     2715       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[FlorianHockmann]

If I'm not mistaken, then this PR does two things which are mostly separate from each other, right? It extends the type definitions for existing types like Vertex, Edge, and VertexProperty and it also adds a new utility method parseVertex which can be used to create a Vertex object where a VertexProperty is directly a property of the object, e.g., vertex.name.

I think it would be good to separate these two things from each other. Could you please create issues for them in our JIRA? Our usual development workflow is to first have an issue there which describes the bug / proposed improvement and afterwards create the PR. This also makes it possible to first discuss the issue before actually implementing it.

In this case, I'm not 100% about the added utility method. It sounds to me like it's a first step in the direction of the Sugar Plugin for Groovy and I'm not sure whether we should implement such syntactic sugar for the GLVs like gremlin-javascript.
But maybe I'm just missing context why this is useful for Javascript / Typescript. You could probably better describe your motivation for this in the JIRA issue.

Sorry for the slight push back here in general. Your Typescript improvements are more than welcome, but I think they are difficult for us to review since we simply lack some experience in this area. That is why I think that following this process helps as it makes it easier for other contributors to better understand the improvements before reviewing them.

[Cole-Greer]

Hi @tien, thanks for putting this together.

I agree with Florian that for this type of change, it is best to start with a JIRA or dev list post before starting implementation.

I do think that consistency between the GLV's is important, but I can also see how this could be a valuable convenience for users. I do think if we are to add this for vertices in JS, we need to do the equivalent for edges.

In terms of naming, an alternative worth considering might be flattenVertex but this isn't something I feel strongly about.

The overall implementation looks good to me, although I wonder if the need to provide explicit cardinality schemas for anything other than singles will limit the convenience upside here. It would be cool if there was an option to try and detect the cardinality of each property, and only force a conversion if the user explicitly specifies an override. Granted this would currently be limited to just single and list, as set is currently blocked by TINKERPOP-3011

In general, if this is something that JS users want in the GLV, then I'm willing to put aside some of the cross-GLV consistency concerns here. I think it's worth a quick dev list thread just to ensure the design is aligned with the community.

[tien]

Hey guys, thanks for the feedbacks, yes that make sense. So I've reduced the scope of the PR change to just improving the TypeScript definition for graph structures: vertex, edge, etc

[tien]

I mainly included the utility function as a demonstration of what a strong TypeScript definition can enable.

As for the benefit of having an utility function like that, with TypeScript I have to do a lot of boilerplates just to extract basic values from vertices, i.e:

const vertices = await g.V().hasLabel("person").toList<PersonVertex>();

const people = vertices.map(vertex => {
  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
  const age = vertex.properties.age.at(0)!.value;

  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
  const name = vertex.properties.name.at(0)!.value;
  
  const addresses = vertex.properties.address.map(x => x.value);

  return {
    age,
    name,
    addresses
  };
});

which with the utility function can be simplified to

const vertices = await g.V().hasLabel("person").toList<PersonVertex>();
const people = vertices.map(vertex => parseVertex(vertex, { address: "list" }));

[tien]

I'm happy to pursue this with a Jira ticket or via the dev list as mentioned if you guys think a utility like this is valuable to be included in the core gremlin lib.

[vkagamlyk]

Hi @tien,
I think this is a good direction for improvement for TinkerPop 5/6 when/if we introduce schema, but now it is important to have similar capabilities in all GLV's.

Perhaps it makes sense to make an example from this code? Or some utility class?

this is really beautiful code, but my opinion is VOTE+0

[tien]

Hey @vkagamlyk, as per my comment here I've removed all none GLV's feature parity changes, so this PR will just be improving the TypeScript definition. No feature changes are involved.

This is the utility function that I've now removed, demonstrating the benefit of strong type definition

export const parseVertex = <
  TVertex extends Vertex,
  TCardinality extends { [P in keyof NonNullable<TVertex['properties']>]?: 'single' | 'list' | 'set' | undefined },
>(
  vertex: TVertex,
  cardinality: TCardinality = {} as TCardinality,
): {
  [P in keyof NonNullable<TVertex['properties']>]: (typeof cardinality)[P] extends 'single'
    ? NonNullable<TVertex['properties']>[P][0]['value']
    : (typeof cardinality)[P] extends 'list'
      ? Array<NonNullable<TVertex['properties']>[P][0]['value']>
      : (typeof cardinality)[P] extends 'set'
        ? Set<NonNullable<TVertex['properties']>[P][0]['value']>
        : NonNullable<TVertex['properties']>[P][0]['value'];
} =>
  Object.fromEntries(
    Object.entries<VertexProperties>(vertex.properties as any).map(([key, value]): any => [
      key,
      (() => {
        switch (cardinality[key]) {
          case 'list':
            return value.map((x) => x.value);
          case 'set':
            return new Set(value.map((x) => x.value));
          case 'single':
          default:
            return value[0].value;
        }
      })(),
    ]),
  ) as any;
  
type ThingVertex = Vertex<"thing", { name: string; alias: string; scores: number }>;

// Raw vertex, for example when calling g.V().next()
const rawVertex: ThingVertex = {
  id: 0,
  label: 'thing',
  properties: {
    name: [{ id: 0, label: 'name', key: 'name', value: 'Foo' }],
    alias: [
      { id: 0, label: 'alias', key: 'alias', value: 'Bar' },
      { id: 0, label: 'alias', key: 'alias', value: 'Baz' },
    ],
    scores: [
      { id: 0, label: 'score', key: 'score', value: 4 },
      { id: 0, label: 'score', key: 'score', value: 20 },
    ],
  },
};

const vertex = parseVertex(rawVertex, {
  // Optional, default to single
  name: 'single',
  alias: 'list',
  scores: 'set',
}); // Fully type safe

console.log(vertex); // Output: { name: 'Foo', alias: ['Bar', 'Baz'], scores: Set [ 4, 20 ] }