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

Incorrect metadata types #666

Closed
2 tasks done
JaneJeon opened this issue Oct 31, 2023 · 4 comments · Fixed by #673
Closed
2 tasks done

Incorrect metadata types #666

JaneJeon opened this issue Oct 31, 2023 · 4 comments · Fixed by #673

Comments

@JaneJeon
Copy link

Prerequisites

  • I'm using the last version.
  • My node version is the same as declared as package.json.

Subject of the issue

The Metadata type exported by metascraper is incorrect.

  interface Metadata {
    author: string;
    date: string;
    description: string;
    image: string;
    publisher: string;
    title: string;
    url: string;
  }

This is what's indicated by the code, but (for obvious reasons) all of these fields are nullable; in fact, some fields may even be missing altogether (if I'm not using, say, metascraper-description).

Steps to reproduce

This is just in the metascraper/index.d.ts code

Expected behaviour

The types should be changed to indicate that these properties are nullable, and that they may not be present.

Actual behaviour

Right now, the types indicate that all of these properties are present at all times and all have string values.

@JaneJeon
Copy link
Author

Oh, lang is missing as well (from metascraper-lang)

@Kikobeats
Copy link
Member

@JaneJeon I will accept a PR fixing this 🙏

@JaneJeon
Copy link
Author

JaneJeon commented Nov 2, 2023

What is the preferred way to resolve this? Just make the output contain every single possible type and make it nullable (and optional)? Or somehow compose types package by package?

(I have no idea how you would go about approach 2 - my typescript-fu is too weak)

@Kikobeats
Copy link
Member

Kikobeats commented Nov 2, 2023

Any rule can return string/number/whatever but also null; null means the value isn't being resolved: https://github.com/microlinkhq/metascraper/blob/master/packages/metascraper/src/get-data.js#L6

I think it could be as simple as:

interface Metadata {
  author: string | null;
  date: string | null;
  description: string | null;
  image: string | null;
  publisher: string | null;
  title: string | null;
  url: string | null;
}

About why lang is missing, it's tricky because every property has its package. The lang will be present if metascraper-lang is is loaded.

What could be nice is to add every property added by any metascraper package with a little description, such as:

interface Metadata {
  /* Get lang property from HTML markup. It will be present when `metascraper-lang` is required */
  lang: string | null
}

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

Successfully merging a pull request may close this issue.

2 participants