Skip to content

Fix & improve match & add matchTag#8

Merged
LukasKalbertodt merged 2 commits into
mainfrom
improve-match
Feb 20, 2025
Merged

Fix & improve match & add matchTag#8
LukasKalbertodt merged 2 commits into
mainfrom
improve-match

Conversation

@LukasKalbertodt
Copy link
Copy Markdown
Member

Based on #7 -> only review the last two commits and wait with merging until #7 is merged. That's why this is a draft.

Unfortunately match was unsound before and could allow getting variables of type T when it should have been T | null. See the relevant commit message for more information. The changes to match are breaking, so this requires a major version bump.

Comment thread src/util.tsx Outdated
Comment thread src/util.tsx Outdated
Comment thread src/util.tsx Outdated
Comment thread src/util.tsx Outdated
Comment thread src/util.tsx Outdated
Unfortunately, `match` had a problem where sometimes it was unsound.
This was not really the fault of `match` as TS has questionable
semantics regarding `Record<string, _>`. The following worked before:

    const foo: number = match("bla", {
        "a": () => 3,
    });

So: no fallback, an incomplete object and still, it returns `number`.
This can of course fail at runtime. The problem is that `Record<X, _>`
works differently for `string` and for unions of strings.

The new version solves this by returning `Out | null` if not all cases
are covered, meaning that for `string` it always returns `Out | null`.

I also removed the `fallback` overload as the same can easily be
achieved via `?? fallback`.

I also added a rudimentary compile-test to make sure the problematic
cases actually cause compile errors.
@LukasKalbertodt LukasKalbertodt marked this pull request as ready for review February 20, 2025 11:24
@LukasKalbertodt LukasKalbertodt merged commit 1e89b91 into main Feb 20, 2025
@LukasKalbertodt LukasKalbertodt deleted the improve-match branch February 20, 2025 11:30
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.

2 participants