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

fix(assets): fix getImage options type #12349

Merged
merged 1 commit into from
Nov 1, 2024

Conversation

norskeld
Copy link
Contributor

Changes

This PR fixes #12319

As I mentioned in the issue, there are two ways of fixing the problem:

  1. Use a type helper to properly handle Omitting from types containing index signatures.
  2. Remove that index signature from the ImageTransform.

I went with the first one since I guess that index signature was put there for a reason. :)

I also applied Simplify. This is to force TypeScript better resolve properties. Not doing that results in the following error when an empty options objects is passed to getImage:

Type '{}' is not assignable to type 'UnresolvedImageTransform'.
  Property 'src' is missing in type '{}' but required in type '{ src: string | ImageMetadata | Promise<{ default: ImageMetadata; }>; inferSize?: boolean | undefined; }'.ts(2322)

With Simplify the error is:

Type '{}' is not assignable to type 'UnresolvedImageTransform'.
  Property 'src' is missing in type '{}' but required in type '{ [x: string]: any; width?: number | undefined; widths?: number[] | undefined; densities?: (number | `${number}x`)[] | undefined; height?: number | undefined; quality?: ImageQuality | undefined; format?: ImageOutputFormat | undefined; src: string | ... 1 more ... | Promise<...>; inferSize?: boolean | undefined; }'.

The latter is rather verbose, but I'd prefer this over the former's actually misleading message.

Testing

Not sure how to add non-fragile or hardcoded tests here.

Docs

This is a fix that doesn't require any changes in docs.

Copy link

changeset-bot bot commented Oct 31, 2024

🦋 Changeset detected

Latest commit: 84426ca

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Oct 31, 2024
Copy link
Member

@Princesseuh Princesseuh left a comment

Choose a reason for hiding this comment

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

Looks great to me! Thank you for contributing!

@Princesseuh Princesseuh merged commit 1fc83d3 into withastro:main Nov 1, 2024
13 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Nov 1, 2024
@norskeld norskeld deleted the fix/getimage-options-type branch November 1, 2024 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

getImage options type is broken
2 participants