Typed IDs

[lazytype]

It would be great to leverage TypeScript to make things like the following an error:

Address.load(viewer, userId)
                     ^^^^^^ wrong kind of id
Address.load(viewer, 42)
                     ^^ shouldn't be able to hard-code IDs

(this is similar to the UID/OID distinction in www)

TL;DR: Go here for a working demonstration.

This would be possible using a somewhat well-known nominal typing trick called "branded" types. The TypeScript handbook demonstrates a few ways to implement this [0], and there is the more recent method using symbols [1].

At a high level, the approaches fall into two buckets:

• Embed the node type into the ID type:

  type ID<T extends NodeType> = { value: number | string ; type: T }

  This approach would require internal code to have to wrap and unwrap the underlying value and for users to treat the ID type as opaque.
• Trick the TS compiler into creating a "sub-type" of string/number:

  type ID<T extends Ent> = (number | string) & { __brand__: T }
  
  function internalCreateID<T extends Ent>(value: number | string): ID<T> {
    return value as ID<T>;
  }

Theoretically, this could also make the internals better-typed as well. The following generated code:

class AddressBase {
  static async loadX<T extends AddressBase>(
    this: new (viewer: Viewer, data: Data) => T,
    viewer: Viewer,
    id: ID,
  ): Promise<T> {
    return (await loadEntX(
      viewer,
      id,
      AddressBase.loaderOptions.apply(this),
    )) as T;  // need type assertion here due to "could be instantiated with a different subtype of constraint" error
  }

might be able to be re-written as this:

class AddressBase {
  static async loadX<T extends AddressBase>(
    this: new (viewer: Viewer, data: Data) => T,
    viewer: Viewer,
    id: ID<T>,
  ): Promise<T> {
    return (await loadEntX(
      viewer,
      id,
      AddressBase.loaderOptions.apply(this),
    ));  // no type assertion needed because `loadEntX` can infer the `Ent` type from `id`
  }

Practically speaking, to make a change like this happen we'd need to make the Ent interface generic with respect to the nodeType and work around TS's inability to express "Self" in a generic class, e.g.

interface Ent<T extends Ent<T, Type>, Type extends string> {
    id: ID<T>; 
    viewer: Viewer;
    privacyPolicy: PrivacyPolicy;
    nodeType: Type;
}

class User extends Ent<User, 'user'> { ... }

It's not the most attractive, but it'd all be generated code that's hidden from the user.

Altogether, here's a working demonstration!. Please let me know your thoughts and I'd be happy to attempt a pull request 🙂.

[0] https://basarat.gitbook.io/typescript/main-1/nominaltyping
[1] https://twitter.com/rauschma/status/1293752309248397315

[lolopinto]

This is definitely interesting from a technical perspective but I'm not sure what problem we'd be solving here. What's the high level issue you're trying to solve?

I think most calls to the API here would be through GraphQL, and eventually REST APIs and the id received would be a base64-encoded string or the uuid.

In those cases, we'd need to work around all this and not get any of the benefits here.

How often do you have the wrong id being passed to a different API?

[lazytype]

The use case I have in mind is when you need want to safely manipulate IDs as part of product logic. Take the following example:

@gqlQuery({ name: "mutual_friends", type: "[User]" })
async mutualFriends(
  @gqlArg("id", { type: GraphQLID }) id: ID,
): Promise<User[]> {
  const [viewerFriendIds, otherFriendIds] = await Promise.all([
    this.queryFriends().queryIDs(),
    User.load(this.viewer, id).then(async (otherUser) =>
      await otherUser?.queryContacts().queryIDs() ?? [],
    ),
  ]);

  const viewerFriendIdSet = new Set(viewerFriendIds);
  const mutualFriendIds = otherFriendIds.filter((id) => viewerFriendIdSet.has(id));
  return await User.loadMany(this.viewer, ...mutualFriendIds);
}

This code type-checks but there's a bug hidden in it.

Spoiler: otherUser?.queryContacts() should be otherUser?.queryFriends().

If IDs were typed then we would've gotten a type error on viewerFriendIdSet.has(id), e.g.
Argument of type 'ID<Contact>' is not assignable to parameter of type 'ID<User>'

Note: The above example could theoretically be done in EntQuery if it supported some more EntQL-like operators like intersection, but I don't think it's that hard to think of examples that even EntQL could not express. I haven't much considered whether it's possible to express the equivalent SQL join needed using only Custom Queries, but I'd assume it wouldn't feel that ergonomic to do so.

[lolopinto]

yah, plan to eventually support more EntQL-ish features but agreed that's independent from the issues here.

[lolopinto]

also, i'm curious: how are you currently using this? is this being used in production? are you just playing around with it?

[lazytype]

I only just learned of Ent and was doing an audit of things I'd like to see if I were to start using it. It would save me a lot of effort in the future from having to build a home-grown framework. I also just have a vested interest in this being the most valuable tool that it can be since I believe the Ent Framework should really be democratized to more than just FB developers.

[ClaudiuCeia]

FWIW, I built something quite similar last year and I chose to have typed IDs which saved me and a couple of my colleagues from embarrassing bugs more than a few times since.

I went with branded types like @lazytype suggests and while there was some overhead I feel like it was overall worth doing. I guess the only difference is that we made the node type a string enum and people need to define a new node type for each schema. So you end up with something like:

export default class EntUserSchema extends EntSchema {  
  config: {
    type: EntIDTypes.USER,
    // ...
  }
  // ...
}

// ---

enum EntIDTypes {
  USER = "user",
}

type EntID<T extends EntIDTypes> = string & { __brand: T } // all IDs are UUIDs 
type EntUserID = EntID<EntIDTypes.USER>;

[lolopinto]

what are the proposals/thoughts for how to integrate with non-ent code?

I'm currently working with an existing codebase and one of the issues I've run into is converting ent ids into strings (what the old API supported)

and what I've been doing is:

function (id: string) {}


const ent: Ent = getEnt();

// toString() is annoying
foo(ent.id.toString());

how does this work with this proposal?

[lazytype]

So I'm assuming this problem exists because Ent IDs can be either number or string and in the number case you have to convert to string. One approach I can think of is to always store the ID as a string and have another field that tracks whether it was originally a number:

interface Ent {
  id: string;
  idType: 'number' | 'string';
  ...
}

Since I'm not familiar with Ent internal details, I couldn't say if this would result in a lot of annoying conversions back into number. As far as I can think of, you'd only need to do so when writing out to the DB?

Another approach is to not really change anything, but instead of doing foo(ent.id.toString()), do foo(`${ent.id}`) to save some keystrokes ¯\_(ツ)_/¯

[lolopinto]

Right now, we encode ids in graphql using base 64 encoding

the id would have the type and underlying id so that could be simplified, what should we send down here?

when base64 ids are disabled, we'd presumably just send the uuid?

[lazytype]

Right, I don't think anything would change on the GraphQL side. When converting the GraphQL ID back into an Ent ID, we should be able to determine the Ent type from the GraphQL node type

[lolopinto]

re: https://twitter.com/rauschma/status/1293752309248397315

https://twitter.com/_bwhitty/status/1293765976840839168

one of the issues I'm concerned about is the complexity here. this just seems more complicated...

[lazytype]

From my experience in TS codebases the implementation details of branded types doesn't really bog down their usage in any way. You can still just treat the values as strings as you normally would, just with the additional helpful property that incompatible ones can't be assigned to each other. I can understand how it can be intimidating for people used to less sophisticated TS usage, but it truly isn't magic and derives from fundamental TS principles. Ideally TS would support this as a first-class pattern, and indeed it's one of the top 10 most upvoted issues, so there is some hope there

[ClaudiuCeia]

I second that.

If you find branding to be a little to restrictive (ie: you can't just do const user = await loadUser(42) because 42 is not an ID type) then you can define the ID type as:

type Flavor<T> = (string | number) & {
  _kind?: T
};

So that would give you typesafety in cases like:

const articleID = await getArticleID(...);
const user = await loadUser(articleID); // Error, article id type doesn't match user id type
const hardcodedUser = await loadUser(42); // number is compatible though

But either way, most people interacting with the codebase wouldn't have to worry about dealing with the underlying types.

[lolopinto]

ok, i'm down to try and sketch it out and see what it'll look like. I don't really have a contributing.md or have a document that lays out the lay of the land. if you wanna give it a go, how do you want to start?

I also have a simple CLA I'd like you to sign if you want to contribute code. you can send me an email a firstname@lastname.com and I can send it to you. I need to automate this eventually but not there yet.

[lazytype]

I think I see this unfolding as these steps:

1. Make the proposed changes locally to Ent and ID in the core package, run the type-checker, try to fix-up type errors and repeat. By the time all the type errors are gone I'll have probably touched and come to understand the relevant parts of the internals.
2. Make the changes needed to codegen. I only have rudimentary experience with Go build environments, but I think I can figure things out without too much direction, hopefully.
3. Update each of the other JS packages. Testing the changes might be tricky. Are you using npm link/yarn link successfully in your workflow?

[lolopinto]

Yah, i've used npm link successfully. should get you far with the examples.

The issue I've run into so far withnpm link is the graphql package is very finicky about the exact same graphql dependency. Haven't had much success with that so far.

[lazytype]

I saw in one package you had a "resolutions" field in the package.json. I'm guessing that means you've already tried using yarn to resolve the issue?

[lolopinto]

I don’t recall. May have just been trying it out

[lazytype]

Just as an FYI I probably won't get to this until at least this weekend or the following

[lolopinto]

I created #611 to track