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

feat: Improve type inference for oEmbed type in TypeScript #113

Merged
merged 4 commits into from Feb 13, 2024

Conversation

r4ai
Copy link
Contributor

@r4ai r4ai commented Feb 3, 2024

Motivation

There are four types of oEmbed: Photo, Video, Rich, and Link. And each of these has different fields that exist depending on the type. Therefore, it seems that the TypeScript type of oEmbed should be appropriately inferred according to the oEmbed type.

// Before
const metadata = await unfurl("https://example.com")
if (!metadata.oEmbed) return

switch (metadata.oEmbed.type) {
  case "photo":
    // url, width and height should exist and not be undefined because these are required fields for photo type
    const url: string = metadata.oEmbed.url       // ❌ Property 'url' does not exist on type '{ type: "photo" | "video" | "link" | "rich"; width?: number | undefined; height?: number | undefined; version?: string | undefined; title?: string | undefined; author_name?: string | undefined; ... 4 more ...; thumbnails?: [...] | undefined; }'
    const width: number = metadata.oEmbed.width   // ❌ Type 'number | undefined' is not assignable to type 'number'
    const height: number = metadata.oEmbed.height // ❌ Type 'number | undefined' is not assignable to type 'number'
    break

  case "video":
  case "rich":
    // html, width and height should exist and not be undefined because these are required fields for video and rich type
    const html: string = metadata.oEmbed.html     // ❌ Property 'html' does not exist on type '{ type: "photo" | "video" | "link" | "rich"; width?: number | undefined; height?: number | undefined; version?: string | undefined; title?: string | undefined; author_name?: string | undefined; ... 4 more ...; thumbnails?: [...] | undefined; }'
    const width: number = metadata.oEmbed.width   // ❌ Type 'number | undefined' is not assignable to type 'number'
    const height: number = metadata.oEmbed.height // ❌ Type 'number | undefined' is not assignable to type 'number'
    break
}
// After
const metadata = await unfurl("https://example.com")
if (!metadata.oEmbed) return

switch (metadata.oEmbed.type) {
  case "photo":
    const url: string = metadata.oEmbed.url       // ✅
    const width: number = metadata.oEmbed.width   // ✅
    const height: number = metadata.oEmbed.height // ✅
    break

  case "video":
  case "rich":
    const html: string = metadata.oEmbed.html     // ✅
    const width: number = metadata.oEmbed.width   // ✅
    const height: number = metadata.oEmbed.height // ✅
    break
}

oEmbed's TypeScript types were implemented based on the following specification.

2.3.4. Response parameters

Responses can specify a resource type, such as photo or video. Each type has specific parameters associated with it. The following response parameters are valid for all response types:

  • type (required)
    The resource type. Valid values, along with value-specific parameters, are described below.
  • version (required)
    The oEmbed version number. This must be 1.0.
  • title (optional)
    A text title, describing the resource.
  • author_name (optional)
    The name of the author/owner of the resource.
  • author_url (optional)
    A URL for the author/owner of the resource.
  • provider_name (optional)
    The name of the resource provider.
  • provider_url (optional)
    The url of the resource provider.
  • cache_age (optional)
    The suggested cache lifetime for this resource, in seconds. Consumers may choose to use this value or not.
  • thumbnail_url (optional)
    A URL to a thumbnail image representing the resource. The thumbnail must respect any maxwidth and maxheight parameters. If this parameter is present, thumbnail_width and thumbnail_height must also be present.
  • thumbnail_width (optional)
    The width of the optional thumbnail. If this parameter is present, thumbnail_url and thumbnail_height must also be present.
  • thumbnail_height (optional)
    The height of the optional thumbnail. If this parameter is present, thumbnail_url and thumbnail_width must also be present.

Providers may optionally include any parameters not specified in this document (so long as they use the same key-value format) and consumers may choose to ignore these. Consumers must ignore parameters they do not understand.

2.3.4.1. The photo type

This type is used for representing static photos. The following parameters are defined:

  • url (required)
    The source URL of the image. Consumers should be able to insert this URL into an element. Only HTTP and HTTPS URLs are valid.
  • width (required)
    The width in pixels of the image specified in the url parameter.
  • height (required)
    The height in pixels of the image specified in the url parameter.

Responses of this type must obey the maxwidth and maxheight request parameters.

2.3.4.2. The video type

This type is used for representing playable videos. The following parameters are defined:

  • html (required)
    The HTML required to embed a video player. The HTML should have no padding or margins. Consumers may wish to load the HTML in an off-domain iframe to avoid XSS vulnerabilities.
  • width (required)
    The width in pixels required to display the HTML.
  • height (required)
    The height in pixels required to display the HTML.

Responses of this type must obey the maxwidth and maxheight request parameters. If a provider wishes the consumer to just provide a thumbnail, rather than an embeddable player, they should instead return a photo response type.

2.3.4.3. The link type

Responses of this type allow a provider to return any generic embed data (such as title and author_name), without providing either the url or html parameters. The consumer may then link to the resource, using the URL specified in the original request.

2.3.4.4. The rich type

This type is used for rich HTML content that does not fall under one of the other categories. The following parameters are defined:

  • html (required)
    The HTML required to display the resource. The HTML should have no padding or margins. Consumers may wish to load the HTML in an off-domain iframe to avoid XSS vulnerabilities. The markup should be valid XHTML 1.0 Basic.
  • width (required)
    The width in pixels required to display the HTML.
  • height (required)
    The height in pixels required to display the HTML.

Responses of this type must obey the maxwidth and maxheight request parameters.

https://oembed.com/#section2

Copy link
Owner

@jacktuck jacktuck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks this looks great

I don't have time to look at the failing tests right now though @r4ai . If you can get them passing i'll merge.

@jacktuck jacktuck changed the title Improve type inference for oEmbed type in TypeScript feat: Improve type inference for oEmbed type in TypeScript Feb 12, 2024
Comment on lines +36 to +37
const oEmbed =
result.oEmbed?.type === "video" ? result.oEmbed : (result.oEmbed as never);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the oEmbed type is "link", width and height do not exist, so it must first be checked that the oEmbed type is "video".

Also, with only expect(result.oEmbed?.type).toEqual("video"), TypeScript cannot infer that the type of result.oEmbed is OEmbedVideo. Therefore, the following type guard is additionally written.

const oEmbed = result.oEmbed?.type === "video" ? result.oEmbed : (result.oEmbed as never)

At this line, Since we have already checked that result.oEmbed?.type === "video" in the above line, the "else" part is regarded as never.

@r4ai
Copy link
Contributor Author

r4ai commented Feb 13, 2024

Thanks for the review!

In the local environment, I could confirm that the test passed successfully, with Node.js version v21.6.1.

$ npm run test

> unfurl.js@5.1.0 test
> npm run lint && port=9000 jest --verbose --runInBand --coverage --coveragePathIgnorePatterns '/test/'


> unfurl.js@5.1.0 lint
> eslint . --ext .ts

ts-jest[versions] (WARN) Version 4.8.4 of typescript installed has not been tested with ts-jest. If you're experiencing issues, consider using a supported version (>=2.7.0 <4.0.0). Please do not report issues in ts-jest if you are using unsupported versions.
(node:172689) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
 PASS  test/oembed/test.ts
  ✓ should noop and not throw for wrong content type (21ms)
  ✓ width/height should be numbers (10ms)
  ✓ should decode entities in OEmbed URL (10ms)
  ✓ should prefer fetching JSON oEmbed (8ms)
  ✓ should upgrade to HTTPS if needed (9ms)
  ✓ should build oEmbed from JSON (7ms)
  ✓ should build oEmbed from XML (7ms)
  ✓ should build oEmbed from XML with CDATA (10ms)

 PASS  test/encoding/test.ts
  ✓ should detect GB2312 charset (HTML 4) and convert to UTF-8 (24ms)
  ✓ should detect GB2312 charset (HTML 5) and convert to UTF-8 (2ms)
  ✓ should detect EUC-JP charset (HTML 5) and convert to UTF-8 (7ms)

 PASS  test/twitter_card/test.ts
  ✓ should build players[] (6ms)
  ✓ should build images[] (5ms)
  ✓ should build apps[] (4ms)
  ✓ should quality relative urls (5ms)
  ✓ should build card (6ms)

 PASS  test/open_graph/test.ts
  ✓ should build videos[] (7ms)
  ✓ should build images[] (3ms)
  ✓ should build audio[] (4ms)
  ✓ should quality relative urls (4ms)
  ✓ should build article[] (4ms)

 PASS  test/general/status-code.test.ts
  ✓ should throw if status code not 200 (19ms)
  ✓ should not throw if status code is 200 (4ms)

 PASS  test/basic/test.ts
  ✓ should handle content which is escaped badly (6ms)
  ✓ should detect title, description, keywords and canonical URL (5ms)
  ✓ should detect title, description, keywords and canonical URL even when they are in the body (3ms)
  ✓ should detect last dupe of title, description and keywords (4ms)
  ✓ should detect last dupe of title, description and keywords (3ms)

 PASS  test/general/options.test.ts
  ✓ should throw bad options error (10ms)
  ✓ should respect oembed (6ms)

 PASS  test/general/content-type.test.ts
  ✓ should throw bad content type error (22ms)

 PASS  test/general/url.test.ts
  ✓ should not throw when provided non-ascii url (3ms)

--------------------|----------|----------|----------|----------|-------------------|
File                |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
--------------------|----------|----------|----------|----------|-------------------|
All files           |    99.52 |    95.31 |      100 |    99.52 |                   |
 index.ts           |     99.5 |    95.31 |      100 |    99.49 |               376 |
 schema.ts          |      100 |      100 |      100 |      100 |                   |
 unexpectedError.ts |      100 |      100 |      100 |      100 |                   |
--------------------|----------|----------|----------|----------|-------------------|
Handlebars: Access has been denied to resolve the property "statements" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Handlebars: Access has been denied to resolve the property "branches" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Handlebars: Access has been denied to resolve the property "functions" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Handlebars: Access has been denied to resolve the property "lines" because it is not an "own property" of its parent.
You can add a runtime option to disable the check or this warning:
See https://handlebarsjs.com/api-reference/runtime-options.html#options-to-control-prototype-access for details
Test Suites: 1 skipped, 9 passed, 9 of 10 total
Tests:       1 skipped, 32 passed, 33 total
Snapshots:   0 total
Time:        1.637s, estimated 2s
Ran all test suites.

@r4ai r4ai requested a review from jacktuck February 13, 2024 09:32
@jacktuck jacktuck merged commit a1b5a97 into jacktuck:master Feb 13, 2024
4 checks passed
Copy link

🎉 This PR is included in version 6.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

2 participants