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: Replace ambiguous headers types with actual types #834

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ryami333
Copy link
Contributor

@ryami333 ryami333 commented Jun 25, 2024

Currently the headers types (both in the context of request/config and response) are ambiguously (any) typed, which prevents Typescript from being able to guard against obvious mis-configuration or mis-use of the response data.

Pull request type

Please check the type of change your PR introduces:-->

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Other (please describe):

How to test this PR

  1. Create an instance of the Storyblok client, and try to configure both string-valued headers, and non-string-valued headers. You should expect a Typescript error when you pass a non-string value (and no error otherwise).
  2. Call a getter method like getStory and inspect the type of response.headers. It should be of type Headers (with .get methods, et al).
const client = new Storyblok({
  headers: {
    'Content-Type': 'application/json',
    'X-Foo': [1, 2, 3], // Error: Type 'number[]' is not assignable to type 'string'
  },
})

client.getStory('xxxxxxx', {}).then((response) => {
  const contentType = response.headers.get('Content-Type') // Type: string | null
})
Screenshot 2024-06-25 at 14 07 15 Screenshot 2024-06-25 at 14 08 20

What is the new behavior?

No new functional behaviour - only changes to types to reflect actual runtime usage.

Other information

None.

Copy link
Contributor Author

@ryami333 ryami333 Jun 25, 2024

Choose a reason for hiding this comment

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

Lots of whitespace/indentation changes because this file had not been correctly formatted with Prettier. See this PR where I set a formatting baseline. Once that is merged I can rebase this PR to make the diffs less noisy.

In the meantime, please hide whitespace changes in code-review.

EDIT: rebased onto main, no more formatting changes.

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 this pull request may close these issues.

1 participant