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: relocate custom condition #8959

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tylersayshi
Copy link
Contributor

@tylersayshi tylersayshi commented Apr 6, 2025

fixes the CI failures added in: #8935

thanks! @beaussan for the help: https://bsky.app/profile/beaussan.io/post/3lm3oodmras2e

apologies for my oversight on adding this originally

Copy link

nx-cloud bot commented Apr 6, 2025

View your CI Pipeline Execution ↗ for commit e82d8a9.

Command Status Duration Result
nx affected --targets=test:sherif,test:knip,tes... ❌ Failed 3m 30s View ↗
nx run-many --target=build --exclude=examples/*... ✅ Succeeded 1m 43s View ↗

☁️ Nx Cloud last updated this comment at 2025-04-08 01:33:43 UTC

Copy link

pkg-pr-new bot commented Apr 6, 2025

More templates

@tanstack/angular-query-devtools-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@8959

@tanstack/angular-query-experimental

npm i https://pkg.pr.new/@tanstack/angular-query-experimental@8959

@tanstack/query-async-storage-persister

npm i https://pkg.pr.new/@tanstack/query-async-storage-persister@8959

@tanstack/query-broadcast-client-experimental

npm i https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@8959

@tanstack/eslint-plugin-query

npm i https://pkg.pr.new/@tanstack/eslint-plugin-query@8959

@tanstack/query-core

npm i https://pkg.pr.new/@tanstack/query-core@8959

@tanstack/query-devtools

npm i https://pkg.pr.new/@tanstack/query-devtools@8959

@tanstack/query-persist-client-core

npm i https://pkg.pr.new/@tanstack/query-persist-client-core@8959

@tanstack/query-sync-storage-persister

npm i https://pkg.pr.new/@tanstack/query-sync-storage-persister@8959

@tanstack/react-query

npm i https://pkg.pr.new/@tanstack/react-query@8959

@tanstack/react-query-devtools

npm i https://pkg.pr.new/@tanstack/react-query-devtools@8959

@tanstack/react-query-next-experimental

npm i https://pkg.pr.new/@tanstack/react-query-next-experimental@8959

@tanstack/react-query-persist-client

npm i https://pkg.pr.new/@tanstack/react-query-persist-client@8959

@tanstack/solid-query

npm i https://pkg.pr.new/@tanstack/solid-query@8959

@tanstack/solid-query-devtools

npm i https://pkg.pr.new/@tanstack/solid-query-devtools@8959

@tanstack/solid-query-persist-client

npm i https://pkg.pr.new/@tanstack/solid-query-persist-client@8959

@tanstack/svelte-query

npm i https://pkg.pr.new/@tanstack/svelte-query@8959

@tanstack/svelte-query-devtools

npm i https://pkg.pr.new/@tanstack/svelte-query-devtools@8959

@tanstack/svelte-query-persist-client

npm i https://pkg.pr.new/@tanstack/svelte-query-persist-client@8959

@tanstack/vue-query

npm i https://pkg.pr.new/@tanstack/vue-query@8959

@tanstack/vue-query-devtools

npm i https://pkg.pr.new/@tanstack/vue-query-devtools@8959

commit: e82d8a9

Copy link

codecov bot commented Apr 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.97%. Comparing base (3319d35) to head (4a64062).
Report is 9 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #8959       +/-   ##
===========================================
+ Coverage   44.09%   60.97%   +16.87%     
===========================================
  Files         201      152       -49     
  Lines        8025     5848     -2177     
  Branches     1770     1579      -191     
===========================================
+ Hits         3539     3566       +27     
+ Misses       4058     1975     -2083     
+ Partials      428      307      -121     
Components Coverage Δ
@tanstack/angular-query-devtools-experimental ∅ <ø> (∅)
@tanstack/angular-query-experimental 88.08% <ø> (ø)
@tanstack/eslint-plugin-query 87.81% <ø> (ø)
@tanstack/query-async-storage-persister 43.85% <ø> (ø)
@tanstack/query-broadcast-client-experimental ∅ <ø> (∅)
@tanstack/query-codemods ∅ <ø> (∅)
@tanstack/query-core 94.77% <ø> (ø)
@tanstack/query-devtools 3.61% <ø> (ø)
@tanstack/query-persist-client-core 57.73% <ø> (ø)
@tanstack/query-sync-storage-persister 84.61% <ø> (ø)
@tanstack/react-query 95.42% <ø> (ø)
@tanstack/react-query-devtools 10.00% <ø> (ø)
@tanstack/react-query-next-experimental ∅ <ø> (∅)
@tanstack/react-query-persist-client 100.00% <ø> (ø)
@tanstack/solid-query 78.48% <ø> (ø)
@tanstack/solid-query-devtools ∅ <ø> (∅)
@tanstack/solid-query-persist-client 100.00% <ø> (ø)
@tanstack/svelte-query 88.15% <ø> (ø)
@tanstack/svelte-query-devtools ∅ <ø> (∅)
@tanstack/svelte-query-persist-client 100.00% <ø> (ø)
@tanstack/vue-query 71.01% <ø> (ø)
@tanstack/vue-query-devtools ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tylersayshi
Copy link
Contributor Author

hm, maybe there's another thing wrong. Not sure why it passed before merging main and failed after 👀

@tylersayshi tylersayshi marked this pull request as draft April 6, 2025 05:41
@TkDodo
Copy link
Collaborator

TkDodo commented Apr 6, 2025

I did the same steps-to-reproduce here and it still fails for me locally:

  • pnpm run clean
  • npx nx run @tanstack/query-core:compile
  • npx nx run @tanstack/react-query:test:lib

@beaussan
Copy link

beaussan commented Apr 6, 2025

Here is my findings so far:

Based on those steps:

  • pnpm run clean
  • npx nx run @tanstack/query-core:compile
  • npx nx run @tanstack/react-query:test:lib

Setting typecheck: { enabled: false }, to the vitest config for react-query, we have more tests running, but still have one error:

image

So, runtime in tests seems to work, maybe it's related to types ?

I may have more time to check in a couple of hours

Edit: life got in the way, maybe I can look into it more tomorrow

@TkDodo
Copy link
Collaborator

TkDodo commented Apr 7, 2025

FYI, I changed the dependencies in nx to have test depend on build until this is sorted, as other PRs can’t complete otherwise:

1771dd2

If this is figured out, please revert this change here 🙏

@TkDodo
Copy link
Collaborator

TkDodo commented Apr 7, 2025

and now we get errors like this:

TypeCheckError: Output file '/home/workflows/workspace/packages/query-persist-client-core/dist-ts/src/index.d.ts' has not been built from source file '/home/workflows/workspace/packages/query-persist-client-core/src/index.ts'.

It’s a pretty messy spot we’re in right now, @tylersayshi I think we should revert the customConditions until this is figured out completely

@tylersayshi
Copy link
Contributor Author

I think we should revert the customConditions until this is figured out completely

I agree. Sorry for the trouble it's caused so far 🫠. I assumed it would be more straightforward

@tylersayshi
Copy link
Contributor Author

added the revert PR: #8973

@github-actions github-actions bot added documentation Improvements or additions to documentation package: svelte-query labels Apr 8, 2025
@tylersayshi tylersayshi force-pushed the move-custom-condition branch from 4a64062 to e82d8a9 Compare April 8, 2025 01:28
@tylersayshi
Copy link
Contributor Author

I did the same steps-to-reproduce here and it still fails for me locally:

  • pnpm run clean
  • npx nx run @tanstack/query-core:compile
  • npx nx run @tanstack/react-query:test:lib

When I run this it succeeds:

  • pnpm run clean
  • npx nx run @tanstack/query-core:compile
  • npx nx run @tanstack/query-persist-client-core:compile
  • npx nx run @tanstack/react-query:test:lib

Is "@tanstack/query-persist-client-core" supposed to automatically compile when compiling query-core?

@TkDodo
Copy link
Collaborator

TkDodo commented Apr 8, 2025

@tylersayshi I’ve changed test:lib to depend on ^build now and that’s why it works.

so, if we change test:lib in nx.json back to "dependsOn": ["^compile"], as it should be, we still get the error. Note that if I do:

  • pnpm run clean
  • npx nx run @tanstack/react-query:test:lib

I get the error, and I can see that dist-ts directories exist in the following packages:

  • query-core
  • query-persist-client-core

That means the compile dependencies are set up correctly. The question is why it’s not picked up.

Also note that the build task writes to the build directory, so it seems like vitest (incorrectly) expects output to be there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment