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 preloaded state type #4078

Merged
merged 2 commits into from Oct 28, 2021
Merged

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented May 3, 2021


name: 🐛 fix PreloadedState type on unknown
about: Fixes a type problem brought up in #4076

PR Type

Does this PR add a new feature, or fix a bug?

Fixes a bug

Why should this PR be included?

Fixes a slight misbehavior of PreloadedState in the case the original state type contained unknown at some point.

Checklist

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Is there an existing issue for this PR?
  • Have the files been linted and formatted?
  • Have the docs been updated to match the changes in the PR?
  • Have the tests been updated to match the changes in the PR?
  • Have you run the tests locally to confirm they pass?

Unfortunately, locally the formatter runs amok and linter/tests don't wanna run. I hope everything is alright.

Bug Fixes

Before, PreloadedState<{state: unknown}> returned { state: never }, now it returns { state: unknown }

How does this PR fix the problem?

Return the original type S in the inner condition instead of never. Seems to be more reasonable.

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 3, 2021

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.

Latest deployment of this branch, based on commit 9ab0a70:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@samarmohan
Copy link

Please format

@phryneas
Copy link
Member Author

Unfortunately, the formatter for this repo is for whatever reason not working correctly on my system and messes up all files. I had pinged @markerikson if he could format over it, but I assume he forgot about it ^^

Mark, can you give this a small "format" please? :)

@vinothvk
Copy link

Can this be merged and released please ? thanks

@garrettm
Copy link

Will this be merged soon?

@markerikson
Copy link
Contributor

Ironically, I just ran into this one myself helping someone out tonight and filed #4201 to fix it.... and then Lenz reminded me this PR exists :)

Merging and publishing 4.1.2 momentarily.

@markerikson markerikson merged commit ef5e57e into reduxjs:4.x Oct 28, 2021
bors bot pushed a commit to ttbud/ttbud that referenced this pull request Dec 10, 2021
[![WhiteSource Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [redux](http://redux.js.org) ([source](https://togithub.com/reduxjs/redux)) | [`4.1.1` -> `4.1.2`](https://renovatebot.com/diffs/npm/redux/4.1.1/4.1.2) | [![age](https://badges.renovateapi.com/packages/npm/redux/4.1.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/redux/4.1.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/redux/4.1.2/compatibility-slim/4.1.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/redux/4.1.2/confidence-slim/4.1.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>reduxjs/redux</summary>

### [`v4.1.2`](https://togithub.com/reduxjs/redux/releases/v4.1.2)

[Compare Source](https://togithub.com/reduxjs/redux/compare/v4.1.1...v4.1.2)

This release fixes a small specific TS types issue where state types that had a nested `unknown` field inside would cause compilation failures when used as the `preloadedState` argument.

#### What's Changed

-   Fix preloaded state type by [@&#8203;phryneas](https://togithub.com/phryneas) in [reduxjs/redux#4078

**Full Changelog**: reduxjs/redux@v4.1.1...v4.1.2

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [WhiteSource Renovate](https://renovate.whitesourcesoftware.com). View repository job log [here](https://app.renovatebot.com/dashboard#github/ttbud/ttbud).
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.

None yet

6 participants