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: createAsyncThunk union return values fall back to allowing only single member #1449

Merged
merged 1 commit into from
Sep 3, 2021

Conversation

phryneas
Copy link
Member

This should fix #1156

@github-actions
Copy link

size-limit report 📦

Path Size
1. entry point: @reduxjs/toolkit (cjs.production.min.js) 12.21 KB (0%)
1. entry point: @reduxjs/toolkit (esm.js) 10.21 KB (0%)
1. entry point: @reduxjs/toolkit/query (cjs.production.min.js) 20.32 KB (0%)
1. entry point: @reduxjs/toolkit/query (esm.js) 17.36 KB (0%)
1. entry point: @reduxjs/toolkit/query/react (cjs.production.min.js) 22.19 KB (0%)
1. entry point: @reduxjs/toolkit/query/react (esm.js) 19.8 KB (0%)
2. entry point: @reduxjs/toolkit (without dependencies) (cjs.production.min.js) 5.55 KB (0%)
2. entry point: @reduxjs/toolkit (without dependencies) (esm.js) 5.53 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (cjs.production.min.js) 9.2 KB (0%)
2. entry point: @reduxjs/toolkit/query (without dependencies) (esm.js) 9.58 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (cjs.production.min.js) 2.36 KB (0%)
2. entry point: @reduxjs/toolkit/query/react (without dependencies) (esm.js) 2.21 KB (0%)
3. createSlice (esm.js) 5.15 KB (0%)
3. createEntityAdapter (esm.js) 5.82 KB (0%)
3. configureStore (esm.js) 5.83 KB (0%)
3. createApi (esm.js) 15.65 KB (0%)
3. createApi (react) (esm.js) 18.03 KB (0%)
3. fetchBaseQuery (esm.js) 10.91 KB (0%)
3. setupListeners (esm.js) 9.78 KB (0%)
3. ApiProvider (esm.js) 16.97 KB (0%)

@netlify
Copy link

netlify bot commented Aug 28, 2021

✔️ Deploy Preview for redux-starter-kit-docs ready!

🔨 Explore the source changes: 5d7a0cc

🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/6129e2b5dcfe91000746fac7

😎 Browse the preview: https://deploy-preview-1449--redux-starter-kit-docs.netlify.app

@codesandbox-ci
Copy link

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 5d7a0cc:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration

@@ -177,7 +177,7 @@ type GetSerializedErrorType<ThunkApiConfig> = ThunkApiConfig extends {
? GetSerializedErrorType
: SerializedError

type MaybePromise<T> = T | Promise<T>
type MaybePromise<T> = T | Promise<T> | (T extends any ? Promise<T> : never)
Copy link
Collaborator

@Shrugsy Shrugsy Aug 30, 2021

Choose a reason for hiding this comment

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

For my own learning, do you know why it works with this change, but not otherwise? I'm guessing it's something to do with an interaction with IsUnknown, but my brain is turning to jelly trying to understand this

Copy link
Member Author

@phryneas phryneas Aug 30, 2021

Choose a reason for hiding this comment

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

https://artsy.github.io/blog/2018/11/21/conditional-types-in-typescript/ - look for this paragraph and a bit above:

All we did is wrap the meat of our logic in a flimsy tortilla of inevitability, since the outer condition A extends any will, of course, always be true

In this case, assume

type Union = 'A' | 'B'

type X = Promise<Union> // will be Promise<'A' | 'B'>
type Y = Union extends any ? Promise<Union> : never // will be Promise<'A'> | Promise<'B'>

we essentially now do it both ways to get to Promise<'A' | 'B'> | Promise<'A'> | Promise<'B'>

Copy link
Collaborator

Choose a reason for hiding this comment

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

aha, so a case of this then: https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types

I'll probably have to encounter this a couple more times before it sinks in to me that it occurs with boolean too (which I guess is 'just' a union of true | false.

Thanks for the explanation & link, I'll go over that article in full to brush up my knowledge

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that is happens with boolean was completely new to me as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

I just have one question about this now: Isn't T extends any ? Promise<T> : never essentially the only type now? T will always extend any so this will always be Promise<T> and not T, right? I'm probably missing something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bfricka it would have to look like this to fit the behaviour you describe:

type MaybePromise<T> = T extends any ? Promise<T> : T

As it is, it's still a union of three possibilities. It might make more sense if you format it a bit differently:

type MaybePromise<T> = 
  | T
  | Promise<T>
  | (T extends any ? Promise<T> : never)

So the fact that T extends any is only relevant for the third scenario. The other unions of T and Promise<T> don't care whether T extends any is true or not

@phryneas phryneas merged commit e3bc1fe into master Sep 3, 2021
@phryneas phryneas deleted the pr/fix-1156 branch September 3, 2021 09:20
@phryneas phryneas added this to the 1.6.2 milestone Sep 17, 2021
bors bot referenced this pull request in ttbud/ttbud Oct 6, 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 |
|---|---|---|---|---|---|
| [@reduxjs/toolkit](https://redux-toolkit.js.org) ([source](https://togithub.com/reduxjs/redux-toolkit)) | [`1.6.1` -> `1.6.2`](https://renovatebot.com/diffs/npm/@reduxjs%2ftoolkit/1.6.1/1.6.2) | [![age](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/compatibility-slim/1.6.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/confidence-slim/1.6.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>reduxjs/redux-toolkit</summary>

### [`v1.6.2`](https://togithub.com/reduxjs/redux-toolkit/releases/v1.6.2)

[Compare Source](https://togithub.com/reduxjs/redux-toolkit/compare/v1.6.1...v1.6.2)

This release fixes several small issues with RTK Query, as well as a regression in the `createAsyncThunk` types and an issue with sourcemap URLs.

#### Changelog

##### RTK Query Fixes

The `isLoading` flag should only ever be `true` on the first run of a hook, but would sometimes briefly flip to `true` on later calls. That should now stay the correct value.

`fetchBaseQuery` should now work properly when used in conjunction with `node-fetch`.

The `BaseQueryApi` object now correctly includes the `extra` argument that was provided when configuring the thunk middleware, if any.

##### Other Fixes

Sourcemap URLs should now be correct, especially for the CommonJS build artifacts.

`createAsyncThunk`'s types have been updated to correctly infer return values when working with enums.

Lots of assorted docs tweaks and updates!

#### What's Changed

-   Add extra to BaseQueryApi by [@&#8203;ricksanchez](https://togithub.com/ricksanchez) in [https://github.com/reduxjs/redux-toolkit/pull/1378](https://togithub.com/reduxjs/redux-toolkit/pull/1378)
-   fix: point sourceMappingURL to correct sourcemaps in build artifacts by [@&#8203;jawadsh123](https://togithub.com/jawadsh123) in [https://github.com/reduxjs/redux-toolkit/pull/1459](https://togithub.com/reduxjs/redux-toolkit/pull/1459)
-   fix: `createAsyncThunk` union return values fall back to allowing only single member by [@&#8203;phryneas](https://togithub.com/phryneas) in [https://github.com/reduxjs/redux-toolkit/pull/1449](https://togithub.com/reduxjs/redux-toolkit/pull/1449)
-   fix `fetchBaseQuery` for usage with `node-fetch` by [@&#8203;phryneas](https://togithub.com/phryneas) in [https://github.com/reduxjs/redux-toolkit/pull/1473](https://togithub.com/reduxjs/redux-toolkit/pull/1473)
-   fix "isLoading briefly flips back to `true`" [#&#8203;1519](https://togithub.com/reduxjs/redux-toolkit/issues/1519) by [@&#8203;phryneas](https://togithub.com/phryneas) in [https://github.com/reduxjs/redux-toolkit/pull/1520](https://togithub.com/reduxjs/redux-toolkit/pull/1520)

**Full Changelog**: reduxjs/redux-toolkit@v1.6.1...v1.6.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, check this box.

---

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).
bors bot referenced this pull request in ttbud/ttbud Oct 6, 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 |
|---|---|---|---|---|---|
| [@reduxjs/toolkit](https://redux-toolkit.js.org) ([source](https://togithub.com/reduxjs/redux-toolkit)) | [`1.6.1` -> `1.6.2`](https://renovatebot.com/diffs/npm/@reduxjs%2ftoolkit/1.6.1/1.6.2) | [![age](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/compatibility-slim/1.6.1)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/@reduxjs%2ftoolkit/1.6.2/confidence-slim/1.6.1)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>reduxjs/redux-toolkit</summary>

### [`v1.6.2`](https://togithub.com/reduxjs/redux-toolkit/releases/v1.6.2)

[Compare Source](https://togithub.com/reduxjs/redux-toolkit/compare/v1.6.1...v1.6.2)

This release fixes several small issues with RTK Query, as well as a regression in the `createAsyncThunk` types and an issue with sourcemap URLs.

#### Changelog

##### RTK Query Fixes

The `isLoading` flag should only ever be `true` on the first run of a hook, but would sometimes briefly flip to `true` on later calls. That should now stay the correct value.

`fetchBaseQuery` should now work properly when used in conjunction with `node-fetch`.

The `BaseQueryApi` object now correctly includes the `extra` argument that was provided when configuring the thunk middleware, if any.

##### Other Fixes

Sourcemap URLs should now be correct, especially for the CommonJS build artifacts.

`createAsyncThunk`'s types have been updated to correctly infer return values when working with enums.

Lots of assorted docs tweaks and updates!

#### What's Changed

-   Add extra to BaseQueryApi by [@&#8203;ricksanchez](https://togithub.com/ricksanchez) in [https://github.com/reduxjs/redux-toolkit/pull/1378](https://togithub.com/reduxjs/redux-toolkit/pull/1378)
-   fix: point sourceMappingURL to correct sourcemaps in build artifacts by [@&#8203;jawadsh123](https://togithub.com/jawadsh123) in [https://github.com/reduxjs/redux-toolkit/pull/1459](https://togithub.com/reduxjs/redux-toolkit/pull/1459)
-   fix: `createAsyncThunk` union return values fall back to allowing only single member by [@&#8203;phryneas](https://togithub.com/phryneas) in [https://github.com/reduxjs/redux-toolkit/pull/1449](https://togithub.com/reduxjs/redux-toolkit/pull/1449)
-   fix `fetchBaseQuery` for usage with `node-fetch` by [@&#8203;phryneas](https://togithub.com/phryneas) in [https://github.com/reduxjs/redux-toolkit/pull/1473](https://togithub.com/reduxjs/redux-toolkit/pull/1473)
-   fix "isLoading briefly flips back to `true`" [#&#8203;1519](https://togithub.com/reduxjs/redux-toolkit/issues/1519) by [@&#8203;phryneas](https://togithub.com/phryneas) in [https://github.com/reduxjs/redux-toolkit/pull/1520](https://togithub.com/reduxjs/redux-toolkit/pull/1520)

**Full Changelog**: reduxjs/redux-toolkit@v1.6.1...v1.6.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, check this box.

---

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.

Typescript: createAsyncThunk type infer not working correctly in 1.6.0 with multiple enum
3 participants