search

TanStack / query

🤖 Powerful asynchronous state management, server-state utilities and data fetching for the web. TS/JS, React Query, Solid Query, Svelte Query and Vue Query.
https://tanstack.com/query
MIT License
42.67k stars 2.92k forks source link

fix(*): change noop function return type from undefined to void #8299

Closed kangju2000 closed 1 week ago

kangju2000 commented 1 week ago

I modify the return type of noop function from () => {} to noop():void for type consistency.

[AS-IS]

[TO-BE]

TkDodo commented 1 week ago

didn’t you just literally change the same thing @manudeli ?

I’m confused

manudeli commented 1 week ago

didn’t you just literally change the same thing @manudeli ?

I’m confused

@kangju2000 check this comment https://github.com/TanStack/query/pull/8294#discussion_r1843688199 please

kangju2000 commented 1 week ago

Sorry for not checking PR https://github.com/TanStack/query/pull/8294#discussion_r1843688199. I tried to change it to void for semantic clarity of "no operation", but as discussed there, we need undefined for practical type safety in Promise chains. I agree with keeping noop(): undefined.

What do you think about adding this comment to help future contributors understand the design decision? @TkDodo @manudeli

/**
 * No-operation function that returns undefined for type safety in Promise chains
 */
function noop(): undefined {
  return undefined
}
manudeli commented 1 week ago

@TkDodo Since noop returning void and functions returning undefined are widely used in all packages,

How about providing both function noop(): void {} and function returnUndefined(): undefined {} in @tanstack/query-core and using them in all packages such as @tanstack/react-query?

saul-atomrigs commented 1 week ago

Quick, simple suggestion: 

export function noop(): void | undefined {}

Is this ok?

TkDodo commented 1 week ago

That's fine if TypeScript likes it everywhere 👍

saul-atomrigs commented 1 week ago

If that makes typescript unhappy, we may just let it be inferred .. 

export function noop(): {
  return undefined
}

up to you guys 😅

kangju2000 commented 1 week ago

@TkDodo Separate noop and returnUndefined functions as reflected in ~f2820ac3774472949a606f6b07e314a96eeb7649~ https://github.com/TanStack/query/pull/8299/commits/a46bd6784bb0b8e5dc0d375e55790933c00b03ea

manudeli commented 1 week ago

@TkDodo Separate noop and returnUndefined functions as reflected in f2820ac

Looking at this comment(https://github.com/TanStack/query/pull/8294#discussion_r1843688199), it seems that TkDodo did not want to implement noop and returnUndefined separately and include them in the package bundle since they are the same implementation in JavaScript.

So, how about implementing it as follows?

// @tanstack/query-core
export function noop(): void {}
export const returnUndefined = noop as () => undefined // re-assert type

// @tanstack/react-query just use @tanstack/query-core's noop, returnUndefined
import { noop, returnUndefined } from '@tanstack/query-core'

// @tanstack/vue-query just use @tanstack/query-core's noop, returnUndefined
import { noop, returnUndefined } from '@tanstack/query-core'

// @tanstack/svelte-query just use @tanstack/query-core's noop, returnUndefined
import { noop, returnUndefined } from '@tanstack/query-core'

if don't want to export noop, returnUndefined in @tanstack/query-core, How about implementing like below

// @tanstack/query-core
export function noop(): void {}
export const returnUndefined = noop as () => undefined // re-assert type

// @tanstack/react-query
export function noop(): void {}
export const returnUndefined = noop as () => undefined // re-assert type

// @tanstack/vue-query
export function noop(): void {}
export const returnUndefined = noop as () => undefined // re-assert type

// @tanstack/svelte-query
export function noop(): void {}
export const returnUndefined = noop as () => undefined // re-assert type
kangju2000 commented 1 week ago

Thank you for the good suggestion! From a bundle size optimization perspective, type re-assertion seems better. I'll try changing it to export const returnUndefined = noop as () => undefined as suggested, and if there are any readability issues, we can consider rolling back to the previous approach.

Additionally, these utility functions seem to fall outside the core responsibilities of @tanstack/query-core and would be better handled through dedicated utility libraries.

nx-cloud[bot] commented 1 week ago

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 9a6220d4cc2362d35c9721a3e30b0c6f1bed9e45. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 2 targets - [`nx affected --targets=test:sherif,test:knip,test:eslint,test:lib,test:types,test:build,build --parallel=3`](https://cloud.nx.app/runs/3BGnQ5FZAX?utm_source=pull-request&utm_medium=comment) - [`nx run-many --target=build --exclude=examples/** --exclude=integrations/**`](https://cloud.nx.app/runs/Jr2aBIB7S3?utm_source=pull-request&utm_medium=comment)

Sent with 💌 from NxCloud.

TkDodo commented 1 week ago

it doesn’t work. please make sure locally that everything compiles properly.

manudeli commented 1 week ago

@kangju2000 You have to run script test:types https://github.com/TanStack/query/blob/main/package.json#L22 in root

kangju2000 commented 1 week ago

@TkDodo

Sorry for the hassle. I've added type overrides for noop only where necessary. Using the overridden noop in the following code causes errors, so I kept it unchanged in solid-query.

// packages/solid-query/src/createQueries.ts:309
let unsubscribe = noop
createComputed<() => void>((cleanup) => {
  cleanup?.()
  unsubscribe = isRestoring() ? noop : subscribeToObserver()
  return () => queueMicrotask(unsubscribe)
})
pkg-pr-new[bot] commented 1 week ago

Open in Stackblitz

More templates

- [@tanstack/query-example-angular-basic](https://pkg.pr.new/template/8578c5ac-750a-42ea-a4f7-8c3fce96f50e) - [@tanstack/query-example-angular-devtools-panel](https://pkg.pr.new/template/58bf4d65-454a-4114-ad5b-5304c3436a25) - [@tanstack/query-example-angular-pagination](https://pkg.pr.new/template/2fc6dc4c-50eb-43fe-8916-601f8e677af0) - [@tanstack/query-example-angular-infinite-query-with-max-pages](https://pkg.pr.new/template/87f6e2fc-cac9-4330-90c2-8c410fc681ef) - [@tanstack/query-example-angular-query-options-from-a-service](https://pkg.pr.new/template/62e7da81-d46e-41ad-aa47-09039c7392d3) - [@tanstack/query-example-angular-router](https://pkg.pr.new/template/2dd91ffb-e4d7-4b78-977e-50ccb4192882) - [@tanstack/query-example-angular-rxjs](https://pkg.pr.new/template/5e578bdc-918b-4f23-8047-f8b4a55a0f76) - [@tanstack/query-example-angular-simple](https://pkg.pr.new/template/3430a812-ffcb-4127-9583-e02a5213c7df) - [@tanstack/query-example-solid-astro](https://pkg.pr.new/template/d1e5d36e-527a-4312-9f2b-032583d5a7d9) - [@tanstack/query-example-solid-basic-graphql-request](https://pkg.pr.new/template/2380cb1c-28ab-417c-ab33-3bd853651d6c) - [@tanstack/query-example-solid-basic](https://pkg.pr.new/template/3b364315-e1ab-4a7e-a6ae-7cb5363e9ce1) - [@tanstack/query-example-solid-default-query-function](https://pkg.pr.new/template/83738a51-fb49-4446-ba0d-e8e62752bd6c) - [@tanstack/query-example-solid-simple](https://pkg.pr.new/template/0a0ac4b4-7f49-473d-8953-b045b784b53e) - [@tanstack/query-example-solid-start-streaming](https://pkg.pr.new/template/db03fe8f-2527-48bc-9e58-22466029c458) - [@tanstack/query-example-react-algolia](https://pkg.pr.new/template/98e4086f-76b0-4450-8bad-86c1fd6e161d) - [@tanstack/query-example-react-auto-refetching](https://pkg.pr.new/template/0e4931c4-0562-4946-b652-32485409dac2) - [@tanstack/query-example-react-basic](https://pkg.pr.new/template/a4c32714-0040-419b-9e45-8bcdac210017) - [@tanstack/query-example-react-basic-graphql-request](https://pkg.pr.new/template/be5a9743-541f-45e7-8dc0-f4c776237aa1) - [@tanstack/query-example-react-default-query-function](https://pkg.pr.new/template/3ae893bc-c94c-422e-969f-3abb7e9619a4) - [@tanstack/query-example-react-devtools-panel](https://pkg.pr.new/template/436a840a-2f67-462d-8221-54d932a30fe7) - [@tanstack/query-example-react-infinite-query-with-max-pages](https://pkg.pr.new/template/b66cfb26-beab-401b-ae38-b0463da942c7) - [@tanstack/query-example-react-load-more-infinite-scroll](https://pkg.pr.new/template/9662dee2-7de8-4364-9d73-9057ccda5f19) - [@tanstack/query-example-react-nextjs](https://pkg.pr.new/template/767b6475-e279-425c-8370-01fd1fcaf41b) - [@tanstack/query-example-react-nextjs-app-prefetching](https://pkg.pr.new/template/632fcb27-a6ae-4e1f-aa88-30f5302bd159) - [@tanstack/query-example-nextjs-suspense-streaming](https://pkg.pr.new/template/6fba67da-94f6-4c51-b0a5-a94f6ff155b9) - [@tanstack/query-example-react-offline](https://pkg.pr.new/template/6ea11937-00c8-4d6b-b58b-a84f94abd120) - [@tanstack/query-example-react-optimistic-updates-cache](https://pkg.pr.new/template/5b19e8b0-13ad-4d81-a16e-4bce6566389e) - [@tanstack/query-example-react-optimistic-updates-ui](https://pkg.pr.new/template/f6bc15bc-640b-4794-8dc0-93a32cc4a0a0) - [@tanstack/query-example-react-pagination](https://pkg.pr.new/template/6c173ddd-1164-4fa5-8694-a499aab30a94) - [@tanstack/query-example-react-playground](https://pkg.pr.new/template/98358a31-38b8-4579-811e-7702a6738794) - [@tanstack/query-example-react-prefetching](https://pkg.pr.new/template/5d864c90-507d-479f-baa9-0c30019ae4fd) - [@tanstack/query-example-react-react-native](https://pkg.pr.new/template/cdb52235-69dc-447c-a247-037240198163) - [@tanstack/query-example-react-router](https://pkg.pr.new/template/fcdde181-4100-4415-91af-3fc963055f7e) - [@tanstack/query-example-react-rick-morty](https://pkg.pr.new/template/b51c9ac1-fc51-4994-81d6-7c3a4453623c) - [@tanstack/query-example-react-shadow-dom](https://pkg.pr.new/template/bf37886a-d3e6-4bb7-ba54-fed2ef200083) - [@tanstack/query-example-react-simple](https://pkg.pr.new/template/5a31b8db-3967-45b8-8af2-ddb54f3c3160) - [@tanstack/query-example-react-star-wars](https://pkg.pr.new/template/263b2378-02b8-4aff-8ada-284f10f04048) - [@tanstack/query-example-react-suspense](https://pkg.pr.new/template/a3455dff-ee86-4a97-8dee-8e6eaf9e58b6) - [@tanstack/query-example-svelte-auto-refetching](https://pkg.pr.new/template/f766528d-f898-492a-bc11-d304f078d176) - [@tanstack/query-example-svelte-basic](https://pkg.pr.new/template/57d32d7f-60c4-4b3c-b233-820216e77eb0) - [@tanstack/query-example-svelte-load-more-infinite-scroll](https://pkg.pr.new/template/69a8d486-2b92-4c08-a862-69e50191dd50) - [@tanstack/query-example-svelte-optimistic-updates](https://pkg.pr.new/template/f2c9ea32-0ab7-401b-8e0d-1ea16b9ed43f) - [@tanstack/query-example-svelte-playground](https://pkg.pr.new/template/afafdeed-5d55-46d7-8b30-0a794adb202c) - [@tanstack/query-example-svelte-simple](https://pkg.pr.new/template/89b70480-1402-4c78-82ed-5e0cd710f9e6) - [@tanstack/query-example-svelte-ssr](https://pkg.pr.new/template/02e31aa8-ab2a-4b61-bc49-c243d89896f7) - [@tanstack/query-example-svelte-star-wars](https://pkg.pr.new/template/c82408ad-b30e-4017-aa37-cb5a5be41f26) - [@tanstack/query-example-vue-2.6-basic](https://pkg.pr.new/template/c4cc1bc8-bdc1-4e41-9cae-665512d4bb93) - [@tanstack/query-example-vue-2.7-basic](https://pkg.pr.new/template/9991dcbf-827b-451c-8dbc-57ff68918856) - [@tanstack/query-example-vue-basic](https://pkg.pr.new/template/b5131e73-2958-418e-a20c-74814663478d) - [@tanstack/query-example-vue-dependent-queries](https://pkg.pr.new/template/417c6300-6872-412f-93b2-abd7ae520962) - [@tanstack/query-example-vue-nuxt3](https://pkg.pr.new/template/6e1f7a7e-9955-4fa9-b501-87088e9d49ac) - [@tanstack/query-example-vue-simple](https://pkg.pr.new/template/74328ec8-e236-495c-b549-c1501e0a9bf1) - [@tanstack/query-example-vue-persister](https://pkg.pr.new/template/e5c0171b-b79a-4934-a41d-47542a6456b0)

@tanstack/angular-query-devtools-experimental

``` pnpm add https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@8299 ```

@tanstack/angular-query-experimental

``` pnpm add https://pkg.pr.new/@tanstack/angular-query-experimental@8299 ```

@tanstack/query-async-storage-persister

``` pnpm add https://pkg.pr.new/@tanstack/query-async-storage-persister@8299 ```

@tanstack/query-broadcast-client-experimental

``` pnpm add https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@8299 ```

@tanstack/eslint-plugin-query

``` pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-query@8299 ```

@tanstack/query-core

``` pnpm add https://pkg.pr.new/@tanstack/query-core@8299 ```

@tanstack/query-devtools

``` pnpm add https://pkg.pr.new/@tanstack/query-devtools@8299 ```

@tanstack/query-persist-client-core

``` pnpm add https://pkg.pr.new/@tanstack/query-persist-client-core@8299 ```

@tanstack/query-sync-storage-persister

``` pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@8299 ```

@tanstack/react-query

``` pnpm add https://pkg.pr.new/@tanstack/react-query@8299 ```

@tanstack/react-query-devtools

``` pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@8299 ```

@tanstack/react-query-next-experimental

``` pnpm add https://pkg.pr.new/@tanstack/react-query-next-experimental@8299 ```

@tanstack/react-query-persist-client

``` pnpm add https://pkg.pr.new/@tanstack/react-query-persist-client@8299 ```

@tanstack/solid-query

``` pnpm add https://pkg.pr.new/@tanstack/solid-query@8299 ```

@tanstack/solid-query-devtools

``` pnpm add https://pkg.pr.new/@tanstack/solid-query-devtools@8299 ```

@tanstack/solid-query-persist-client

``` pnpm add https://pkg.pr.new/@tanstack/solid-query-persist-client@8299 ```

@tanstack/svelte-query-devtools

``` pnpm add https://pkg.pr.new/@tanstack/svelte-query-devtools@8299 ```

@tanstack/svelte-query

``` pnpm add https://pkg.pr.new/@tanstack/svelte-query@8299 ```

@tanstack/svelte-query-persist-client

``` pnpm add https://pkg.pr.new/@tanstack/svelte-query-persist-client@8299 ```

@tanstack/vue-query

``` pnpm add https://pkg.pr.new/@tanstack/vue-query@8299 ```

@tanstack/vue-query-devtools

``` pnpm add https://pkg.pr.new/@tanstack/vue-query-devtools@8299 ```

commit: 9a6220d

codecov[bot] commented 1 week ago

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 62.69%. Comparing base (fadfbde) to head (9a6220d). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/TanStack/query/pull/8299/graphs/tree.svg?width=650&height=150&src=pr&token=jqEbswkEDQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack)](https://app.codecov.io/gh/TanStack/query/pull/8299?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) ```diff @@ Coverage Diff @@ ## main #8299 +/- ## ========================================= + Coverage 0 62.69% +62.69% ========================================= Files 0 135 +135 Lines 0 4798 +4798 Branches 0 1351 +1351 ========================================= + Hits 0 3008 +3008 - Misses 0 1548 +1548 - Partials 0 242 +242 ``` | [Components](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | Coverage Δ | | |---|---|---| | [@tanstack/angular-query-devtools-experimental](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/angular-query-experimental](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `88.25% <100.00%> (∅)` | | | [@tanstack/eslint-plugin-query](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/query-async-storage-persister](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `43.85% <100.00%> (∅)` | | | [@tanstack/query-broadcast-client-experimental](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/query-codemods](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/query-core](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `93.69% <100.00%> (∅)` | | | [@tanstack/query-devtools](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `4.78% <ø> (∅)` | | | [@tanstack/query-persist-client-core](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `57.73% <ø> (∅)` | | | [@tanstack/query-sync-storage-persister](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `84.61% <0.00%> (∅)` | | | [@tanstack/react-query](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `95.54% <ø> (∅)` | | | [@tanstack/react-query-devtools](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `10.00% <ø> (∅)` | | | [@tanstack/react-query-next-experimental](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/react-query-persist-client](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `100.00% <ø> (∅)` | | | [@tanstack/solid-query](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `78.20% <ø> (∅)` | | | [@tanstack/solid-query-devtools](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/solid-query-persist-client](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `100.00% <ø> (∅)` | | | [@tanstack/svelte-query](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `87.33% <0.00%> (∅)` | | | [@tanstack/svelte-query-devtools](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | | | [@tanstack/svelte-query-persist-client](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `100.00% <ø> (∅)` | | | [@tanstack/vue-query](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `71.45% <ø> (∅)` | | | [@tanstack/vue-query-devtools](https://app.codecov.io/gh/TanStack/query/pull/8299/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=TanStack) | `∅ <ø> (∅)` | |

🚨 Try these New Features: