Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WithContext<T> should be assignable to T #158

Closed
AeonFr opened this issue Jul 12, 2021 · 5 comments · Fixed by #160 · May be fixed by PaulaSalinoRibeiro/my-blog#5
Closed

WithContext<T> should be assignable to T #158

AeonFr opened this issue Jul 12, 2021 · 5 comments · Fixed by #160 · May be fixed by PaulaSalinoRibeiro/my-blog#5

Comments

@AeonFr
Copy link

AeonFr commented Jul 12, 2021

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.

@AeonFr AeonFr changed the title [Help] Type errors using ItemList after updating to 0.8 Type errors using ItemList after updating to 0.8 Jul 12, 2021
@Eyas
Copy link
Collaborator

Eyas commented Jul 12, 2021

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
Copy link
Collaborator

Eyas commented Jul 12, 2021

@AeonFr
Copy link
Author

AeonFr commented Jul 13, 2021

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

@AeonFr AeonFr closed this as completed Jul 13, 2021
@Eyas
Copy link
Collaborator

Eyas commented Jul 13, 2021

Great to hear this was resolved.

I see how this would have happened:

  • Typescript typically ignores excess properties (unless specified in a literal, to help prevent typos), so assigning a WithContext<T> to T should have worked.

  • 0.7.4 added support for a @graph nodes, changing the type of WithContext:

    -   export declare type WithContext<T extends Thing> = T & {
    +   export declare type WithContext<T extends Thing> = Graph | (T & {
            "@context": "https://schema.org";
    +   });
    -   };
    +   export interface Graph {
    +       "@context": "https://schema.org";
    +       "@graph": readonly Thing[];
    +   }

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 Eyas reopened this Jul 13, 2021
@Eyas Eyas changed the title Type errors using ItemList after updating to 0.8 WithContext<T> should be assignable to T Jul 13, 2021
@Eyas Eyas closed this as completed in #160 Jul 13, 2021
@Eyas
Copy link
Collaborator

Eyas commented Jul 13, 2021

Fixed in 0.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants