google / schema-dts

JSON-LD TypeScript types for Schema.org vocabulary
Apache License 2.0
889 stars 33 forks source link

WithContext<T> should be assignable to T #158

Closed AeonFr closed 3 years ago

AeonFr commented 3 years ago

After updating from 0.7 to 0.8, I get errors with this code:

const list: WithContext<ItemList> = {
    "@context": "https://schema.org",
    "@type": "ItemList",
    itemListElement: articleList.map((articleItem, i) =>
      getArticleItemStructuredData({
        articleItem,
        position: i + 1,
      })
    ),
}

The property @type gives the following error: Type '"ItemList"' is not assignable to type '"OfferCatalog"'

I would be able to bypass this error by importing and using ItemListLeaf instead of ItemList, but this interface is not exported. For now my workaround was to recreate the interface (based on the sourcecode, I use Omit<> to deny all members except ItemListLeaf from the intersection).

type ItemListLeaf = Exclude<
  ItemList,
  BreadcrumbList | OfferCatalog | HowToStep | HowToSection
>;

Notice that when I run the output of this code in the Rich Results Test or in the Structured Data Testing Tool, I don't get any errors, that's why I think the problem might not be in my original interface, but in the way the typings are built.

Eyas commented 3 years ago

Thanks for reporting this. I think I'll need to see the exact signature of getArticleItemStructuredData. I can't repro this on my own:

// typescript 4.5.3
// schema-dts 0.8.3
import { WithContext, ItemList, Article } from 'schema-dts';

const articleList: string[] = [];
function getArticleItemStructuredData({
  articleItem: string,
  position: number
}): Article {
  return {
    '@type': 'Article'
  };
}

const list: WithContext<ItemList> = {
  '@context': 'https://schema.org',
  '@type': 'ItemList',
  itemListElement: articleList.map((articleItem, i) =>
    getArticleItemStructuredData({
      articleItem,
      position: i + 1
    })
  )
};

The error, "ItemList"' is not assignable to type '"OfferCatalog", sounds like a red herring. But it might be because your itemListElement type mismatched, and TS wasn't sure which type union item to complain about.

Eyas commented 3 years ago

See https://stackblitz.com/edit/typescript-seupf8?file=index.ts

AeonFr commented 3 years ago

Looks like the error comes from the fact that I'm returning WithContext<Article> instead of Article in the getArticleItemStructuredData function.

Previously this didn't raised any issues, but now it does.

Declaring @context at multiple levels is not considered "invalid" by the google validation tools, so we rolled with it for simplicity. But I understand that it's not the intended way to build schemas.

Thanks for the help. And thanks for the good work maintaining this library

Eyas commented 3 years ago

Great to hear this was resolved.

I see how this would have happened:

and TypeScript now correctly notes that Graph isn't assignable to Article.

I think including Graph in WithContext<> is a mistake, especially since Graph is exported itself as-is. But now I'm not sure why I did that to begin with.

Eyas commented 3 years ago

Fixed in 0.9.0