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

test!: tests for util module #15

Open
wants to merge 1 commit into
base: chore/immer-v10
Choose a base branch
from

Conversation

johannes-lindgren
Copy link
Collaborator

@johannes-lindgren johannes-lindgren commented Aug 23, 2024

I am creating unit tests for the util module. In the process, I discovered a couple of descrepencies between the source code in util.ts and TypeScript. Fixing these are technically breaking changes:

  • isJSONObject should return false for null. This was easy to miss because 1) JavaScript returns object for typeof null, and 2) TypeScript does not check that the return value is consistent with the the type predicate. The new unit tests detects this.
  • There's no need for an else branch in toYDataType. If there are no type errors, it will never return undefined, so we can remove undefined from the return type. While this function is not exposed by the library (index.ts), it might be a breaking change.
  • Remove any from the argument type of toPlainValue.
  • Changing the return type of notImplemented to never; since it always throws an exception, it never returns.

I plan to release these under the next major version, where we also bump the peer dependency on immer to v10 (#13).

@johannes-lindgren johannes-lindgren self-assigned this Aug 23, 2024
@johannes-lindgren johannes-lindgren changed the base branch from main to chore/immer-v10 August 23, 2024 16:02
@johannes-lindgren johannes-lindgren requested a review from sep2 August 26, 2024 21:28
@johannes-lindgren johannes-lindgren marked this pull request as ready for review August 26, 2024 21:28
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