prismicio / prismic-types

Type definitions for Prismic content, models, and APIs
https://prismic.io/docs/technologies/javascript
Apache License 2.0
11 stars 4 forks source link

Libraries using `@prismicio/types` with different versions results in type errors #16

Closed angeloashmore closed 3 years ago

angeloashmore commented 3 years ago

Versions

Reproduction

This is difficult to reproduce so I'll share a general proof.

enum A {
    foo = "foo",
}

enum B {
    foo = "foo",
}

const val: A.foo = B.foo;
// [tsserver 2322] [E] Type 'B' is not assignable to type 'A'.

Despite A.foo and B.foo having equivalent literal values, TypeScript treats them as distinct types.

In the types library, this scenario happens when dependent libraries resolve different @prismicio/types installations. More specifically, when the RichTextNodeType enum exists in more than one location, TypeScript sees them as incompatible.

The following node_modules structure produces this:

This occurs when packages resolve to different versions of @prismicio/types, thus requiring dependent packages to install their own version of @prismicio/types.

Steps to reproduce

I'm having trouble finding reproducible steps that cause this, but indeed it happens:

image

What is expected?

Type errors do not occur when multiple versions of @prismicio/types are installed. Type errors due to structural differences between versions are expected.

What is actually happening?

Multiple versions cause enum incompatibilities. See the above example errors.

angeloashmore commented 3 years ago

One solution is to not use an enum. Enum-like types exist, such as a const object.

Remember that most enums, including RichTextNodeType, are exported as a public API for libraries like @prismicio/helpers. Thus, replacing it with a string union is not ideal as that would require users to type the strings by hand.

Before:

declare const enum RichTextNodeType {
  heading1 = "heading1",
  heading2 = "heading2",
  heading3 = "heading3",
  heading4 = "heading4",
  heading5 = "heading5",
  heading6 = "heading6",
  paragraph = "paragraph",
  preformatted = "preformatted",
  strong = "strong",
  em = "em",
  listItem = "list-item",
  oListItem = "o-list-item",
  list = "group-list-item",
  oList = "group-o-list-item",
  image = "image",
  embed = "embed",
  hyperlink = "hyperlink",
  label = "label",
  span = "span",
}

After:

declare type RichTextNodeType = {
  heading1: "heading1";
  heading2: "heading2";
  heading3: "heading3";
  heading4: "heading4";
  heading5: "heading5";
  heading6: "heading6";
  paragraph: "paragraph";
  preformatted: "preformatted";
  strong: "strong";
  em: "em";
  listItem: "list-item";
  oListItem: "o-list-item";
  list: "group-list-item";
  oList: "group-o-list-item";
  image: "image";
  embed: "embed";
  hyperlink: "hyperlink";
  label: "label";
  span: "span";
};

This resolves the type error shown in the first comment:

const A = {
    foo: "foo",
} as const;

const B = {
    foo: "foo",
} as const;

const val: typeof A["foo"] = B.foo;

The ergonomics of this example need to be improved, but it shows two different types with identical values are compatible.

lihbr commented 3 years ago

I like the above, tricky one indeed as we cannot ensure that same version of types is being installed for everyone.

angeloashmore commented 3 years ago

Yeah, not using an enum is not ideal but I think using an object like above is the next best option. On the up side, it might take up less bytes than the generated enum output.

Are you good moving forward with this change?

lihbr commented 3 years ago

Yup! Will work on that this afternoon~