Generic Delta<T>?

[alecgibson]

The AttributeMap interface currently looks like this:

interface AttributeMap {
  [key: string]: unknown;
}

While this is technically correct, it can be a bit of a pain to work with when accessing attributes programmatically, since you're constantly forced to cast (which can also be error-prone, since we're actually operating outside of type safety):

const headerLevel = op.attributes.header as number;

The easiest (but not type-safe) thing is obviously to change to:

interface AttributeMap {
  [key: string]: any;
}

But use of unknown was an active choice made in #73 (which is fair enough).

Instead, it would be nice to let consumers define their own interfaces, for both improved type-safety and convenience:

interface MyAttributes extends AttributeMap {
  [key: string]: unknown;
  bold?: boolean;
  italic?: boolean;
  myOtherCustomAttribute?: string;
  // etc.
}

interface MyOp extends Op {
  attributes?: MyAttributes
}

This would require us to be able to pass a type argument to Delta:

const isBold = delta.ops[0].attributes.bold; // Automatically narrowed to `boolean`
const unknownProp = delta.ops[0].attributes.unknownProp; // Keeps `unknown` typing

I started trying to put together a PR, but realised it's unfortunately not quite as simple as I'd anticipated, since Delta objects interact with other Delta objects (which is their whole point), and they may or may not share the same Op type.

I guess in theory you could still have things like:

class Delta<O extends Op = Op> {
  compose(other: Delta<T>): Delta<T & O>;
}

But before I launched into this, I thought I'd gather opinions first. Thoughts?

[luin]

Hey @alecgibson 👋,

Thanks for raising this up! I think in practice, you would get delta data from your database and you still have to perform a force casting (Delta<unknown> as Delta<MyOp>) anyway so I don't think it worth the effort to make it generic.

[alecgibson]

My point was more that you can simplify your casting by moving the cast to the Delta level, so you cast just once (on data retrieval), and then after that you're working with a well-typed object, and don't have to cast every time you access an attribute.

[luin]

While this is indeed convenient, the actual data you get may not necessarily conform to your type definition (e.g. due to bugs or data format upgrades), and supporting generics can easily lead developers to overlook this issue. In practice, I'd recommended using typeof attributes.header === 'number' to narrow down the type, although as you said, it is more verbose. Another issue I'm considering is the support for the embed format (op.insert.image). If we provide generics for Op, then it would make no sense not to provide generics for the embed format as well.

[alecgibson]

I'll admit embeds do complicate things, and yes I'd agree we'd want generics for that, too if we did attributes.

If there's no appetite for this, that's fair enough. Thought I'd just float the idea because I know I get bored of constantly casting, but maybe I'm just lazier than most 😛

[luin]

For me I tend to use type guards for attributes and type force casts for embed values as attributes are generally simple values. Probably you may want to define a custom utility function getAttributes(op: Op): MyAttributes to avoid casting.