Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

refactor!: Remove any type from storage #383

Merged
merged 1 commit into from
Aug 16, 2022
Merged

refactor!: Remove any type from storage #383

merged 1 commit into from
Aug 16, 2022

Conversation

mxdvl
Copy link
Contributor

@mxdvl mxdvl commented Aug 4, 2022

What does this change?

Remove any types from the storage API.

This is technically a breaking change, as TypeScript consumers will have to update their code to handle the change.

Why?

This caused an issue in Production recently, where the type that became returned as a number instead of a string.

@mxdvl mxdvl requested a review from a team as a code owner August 4, 2022 13:07
@coveralls
Copy link

coveralls commented Aug 4, 2022

Coverage Status

Coverage remained the same at 100.0% when pulling 0d9ab98 on mxdvl/no-any into 9e86742 on main.

@mxdvl mxdvl force-pushed the mxdvl/no-any branch 2 times, most recently from 05bc766 to 0c9a0a6 Compare August 4, 2022 13:46
Copy link

@ob6160 ob6160 left a comment

Choose a reason for hiding this comment

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

great idea, makes sense to me 👍

@mxdvl mxdvl changed the title fix: Remove any types fix!: Remove any types Aug 4, 2022
@mxdvl mxdvl changed the title fix!: Remove any types refactor!: Remove any types Aug 4, 2022
@mxdvl mxdvl changed the title refactor!: Remove any types refactor!: Remove any type from storage Aug 4, 2022
The `any` is very loose, and consumers
should make appropriate decision about
the data, so `unknown` is more appropriate.

The motivation comes from a bug that made its
way into production recently.
@mxdvl mxdvl enabled auto-merge (squash) August 16, 2022 15:56
@mxdvl mxdvl merged commit 9e6195f into main Aug 16, 2022
@mxdvl mxdvl deleted the mxdvl/no-any branch August 16, 2022 15:57
@github-actions
Copy link

🎉 This PR is included in version 7.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants