Skip to content

Don't widen an all-optional selector result to any - #776

Merged
markerikson merged 2 commits into
reduxjs:masterfrom
veksa:fix/if-unknown-weak-types
Aug 10, 2026
Merged

Don't widen an all-optional selector result to any#776
markerikson merged 2 commits into
reduxjs:masterfrom
veksa:fix/if-unknown-weak-types

Conversation

@veksa

@veksa veksa commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

With strictNullChecks: false, a selector whose result has only optional properties loses its type:

interface Required { a: boolean }
interface Optional { a?: boolean }   // the only difference

createSelector([(s: { x: Required }): Required => s.x], v => v)
// => Selector<..., Required>   ✅

createSelector([(s: { x: Optional }): Optional => s.x], v => v)
// => Selector<..., any>        ❌

Nothing warns about it — the combiner argument just arrives as any and everything built on top quietly degrades. Most state slices have optional fields, so this hits a lot of selectors.

The cause is IfUnknown: unknown extends T isn't a test for unknown. Without strictNullChecks, undefined inhabits every type, so a type with no required properties demands nothing and unknown is assignable to it — it gets mistaken for unknown and replaced by the any fallback. Checking keyof instead separates the two and reads the same under both settings.

One caveat: {} and unknown stay mutually assignable without strictNullChecks and neither has keys, so a result of exactly {} is still widened. That's unchanged from today, and documented on the type.

Tests cover the helpers and createSelector end to end. They run twice — under strict via the existing typecheck, and under strictNullChecks: false via a new tsconfig, since the bug doesn't reproduce under strict. It's wired into type-check, so test already picks it up.

@netlify

netlify Bot commented Aug 10, 2026

Copy link
Copy Markdown

Deploy Preview for reselect-docs canceled.

Name Link
🔨 Latest commit 009ac0c
🔍 Latest deploy log https://app.netlify.com/projects/reselect-docs/deploys/6a79ad925e55e10008188780

@codesandbox-ci

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@EskiMojo14

Copy link
Copy Markdown
Contributor

if you have strictNullChecks off, are you even using Typescript? 😄

@veksa

veksa commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@EskiMojo14

fair 😄 - it's a huge codebase still in the middle of that migration, so the flag isn't off by choice.

either way unknown extends T isn't a test for unknown - under strict that's just indistinguishable, so behaviour there is unchanged. and it fails silently: you don't get an error, you get any.

@markerikson

Copy link
Copy Markdown
Contributor

Yeah, our general policy for all Redux-related libs is we require use of strictNullChecks: true to work properly, and anyone who turns that off is on their own for any types issues that pop up.

This PR by itself seems passable at first glance, I'm just not sure I want to actually add a change that relates to strict checks being off.

@andr1o

andr1o commented Aug 10, 2026

Copy link
Copy Markdown

@markerikson

Please Please consider adding this change, I can relate very much to @veksa 's request. Huge code base, we're in the middle of migration🙏🙏🙏

@markerikson

Copy link
Copy Markdown
Contributor

Yeah, that's fair. And I definitely appreciate hearing that other people are experiencing this issue and would benefit from the fix (and also that you're migrating to fix things :) )

I think we have some other fixes that have been merged and not released. I'll land this, and I'll look at trying to get out a new version later today.

@markerikson
markerikson merged commit 032ca77 into reduxjs:master Aug 10, 2026
24 checks passed
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.

4 participants