test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

test:lib task-caching w/ Nx - #5116

Merged
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching
Mar 14, 2023
Merged

test:lib task-caching w/ Nx#5116
TkDodo merged 20 commits into
TanStack:mainfrom
ZackDeRose:test-lib-caching

Conversation

@ZackDeRose

@ZackDeRoseZackDeRose commented Mar 11, 2023

Copy link
Copy Markdown
Contributor

What This Does

  • Implements local task-caching [currently] only for the test:lib task (with Nx)
  • I also update the root test-lib script to run with nx's run-many command, as well as the Test workflow in .github/workflows/pr.yml
    • While not directly affected, the Run Test step of the ci pipeline is adjusted as well as test:ci will call pnpm run test:lib
    • My testing shows a 20s time boost to the Test job of the pr pipeline (see Task caching ZackDeRose/query#1 (comment) for my findings)
  • Note that any test:lib:dev commands are not affected and work exactly as they did before

What this PR doesn't do, but could!

I wanted to make this PR bite-sized (as opposed to my last one!) but would be happy to expand this with more task-caching features:

  • I believe most of the "testing" tasks could be cached in a similar way (this would mainly take a few lines to the nx.json file, and some coordination to make sure I correctly understand what files are created by those tasks, and which files should/shouldn't invalidate the cache)
  • builds similarly should be cacheable without too much more work
  • We could introduce distributed caching (w/ Nx Cloud) - this would mainly be there to enable caching across github action runs [open source projects get a free unlimited license to Nx Cloud]
    • I'd recommend a public read-only key along with a private write key to be stored in github secrets (happy to help set that up)
    • alternatively, we can persist the node_modules/.cache between ci runs via the workflow yml files, but that's much more manual process (and we'd have to eventually introduce pruning before this cache just gets too large)
  • We can introduce distributed task execution to further parallelize the ci workflows

@TkDodo (or others!) let me know if any of the above would be appealing! I'd be happy to work on introducing any of these (either in this PR or in future PRs)

[Also note that I work full-time for Nx!]

@vercel

vercelBot commented Mar 11, 2023

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
NameStatusPreviewCommentsUpdated
query⬜️ Ignored (Inspect)Mar 11, 2023 at 11:52AM (UTC)

@codesandbox-ci

codesandbox-ciBot commented Mar 11, 2023

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.

Latest deployment of this branch, based on commit 7995733:

SandboxSource
@tanstack/query-example-react-basic-typescriptConfiguration
@tanstack/query-example-solid-basic-typescriptConfiguration
@tanstack/query-example-svelte-basicConfiguration
@tanstack/query-example-vue-basicConfiguration

@codecov-commenter

codecov-commenter commented Mar 11, 2023

Copy link
Copy Markdown

Codecov Report

Patch and project coverage have no change.

Comparison is base (b8b0562) 91.90% compared to head (7995733) 91.90%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@ Coverage Diff @@## main #5116 +/- ##
=======================================
Coverage 91.90% 91.90% =======================================
Files 111 111 Lines 4188 4188 Branches 1083 1083 =======================================
Hits 3849 3849 Misses 318 318 Partials 21 21 

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Comment threadnx.json
@TkDodo

Copy link
Copy Markdown
Collaborator

love the idea of nx-cloud ❤️

@ZackDeRose
ZackDeRose requested a review from TkDodoMarch 14, 2023 05:55
@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Hey @TkDodo! Looks like this failed in CI due to some flakiness (the error comes right after a await sleep(20) - and I confirmed after pulling down the new changes that this passes locally.)

Can we give this another try? Also let me know if anything else you want to see adjusted in this branch!

@TkDodo

Copy link
Copy Markdown
Collaborator

Yeah I'm trying to fix flakiness as I see it. Some tests are not written in a robust way 😅

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I'm trying to fix flakiness as I see it

That's great. If there's an effort to go after these please let me know! I think this one for example, we should use waitFor instead!

@TkDodo

Copy link
Copy Markdown
Collaborator

yeah all the newer tests are written with waitFor instead of sleeps:

constrendered=renderWithClient(queryClient,<Page/>)
constfetchBtn=rendered.getByRole('button',{name: 'refetch'})
awaitwaitFor(()=>rendered.getByText('data: 1'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 2'))
fireEvent.click(fetchBtn)
awaitwaitFor(()=>rendered.getByText('data: 3'))

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Yeah I noticed that! I like that alot. Seems like there might some potential for flakiness based on this timing out, but I've actually never had a test fail on me because of that in any of the testing I've looked at for this project so far!

@TkDodo

Copy link
Copy Markdown
Collaborator

Just watched your video - amazing, thank you so much for this ❤️ . I'll merge the PR and then merge it to alpha. Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

@TkDodo
TkDodo merged commit 19a8e98 into TanStack:mainMar 14, 2023
@TkDodo

Copy link
Copy Markdown
Collaborator

please let me know if I fixed that correctly on alpha 😅

ac31fc0

@TkDodo

Copy link
Copy Markdown
Collaborator

@ZackDeRose I can now see some post-install task failing in our pipelines:
Screenshot 2023-03-14 at 09 34 16

it just logs this error and it still succeeds, but the install times seem higher now (27s vs 11s before)

@TkDodo

Copy link
Copy Markdown
Collaborator

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

@ZackDeRose

ZackDeRose commented Mar 14, 2023

Copy link
Copy Markdown
ContributorAuthor

okay never mind the longer times, this was just a cache miss on pnpms side. It's back to 11s with a warm cache. But do you know what the error is?

Ah yes - the Nx Daemon is a daemon process that runs in the background to eagerly determine Nx project configuration whenever files change. Looks like the daemon stopped because of an issue parsing pnpm's lockfile.

I'll bring this to our core engineers, as we should be handling this better - but the daemon failing shouldn't be considered a failing event. If it ever does fail, it should just restart itself the next time Nx would need it!

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo - I'll be tracking this here: nrwl/nx#15656

@JamesHenry

Copy link
Copy Markdown

Just FYI NX_DAEMON=false environment variable could be used in the immediate term if the issues is daemon specific

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

Thanks @JamesHenry! I'll get something up with this added now :)

@ZackDeRose

Copy link
Copy Markdown
ContributorAuthor

@TkDodo some quick fixes:

#5128 to fix the error messages re: the nx daemon
#5129 quick adjustment to alpha branch

Whatever you wanna do next, I'm game. adding caching for the lint task, or going nx cloud for what we have already 🎉

Let's go for adding the rest of our tasks first! I think that should actually be pretty easy! I'll send you a PR soon.

Let's plan for Nx cloud following that because we may have to coordinate that a bit!

TkDodo added a commit to paul-sachs/query that referenced this pull request Mar 15, 2023
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
TkDodo added a commit that referenced this pull request Mar 15, 2023
…#4352)
* feat(devtools): enable setting loading/error via devtools
* Some cleanup
* refactor: use flex-gap to align buttons
* refactor: fix linter and dropdown reset
* refactor: operate directly on activeQuery
* Change buttons to toggle states
* Sneak some queryState into meta
* Added test for error and loading
* Fix lint
* Fix prettier formatting
* chore: releases should run on alpha/beta as well
* chore: extract package validation to an extra script (#5039)
* chore: extract package validation to an extra script
and run it during CI
* chore: add missing `build:types` script to solid-query
* docs: update link for v2 docs (#5044)
* docs: update link for v2 docs
* Update README.md
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.12
* fix(react-query-devtools): do not stretch query status label (#5063)
Do not stretch query status label (fresh, fetching, paused, stale, inactive) shown on Query Details view.
* release: v4.24.13
* fix(react-query-devtools): add 'use client' directive to disable SSR (#5041)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.24.14
* feat(core): re-export matchQuery from utils (#5070)
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* release: v4.25.0
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache (#5075)
* feat(query-core): Add global onSettled callbacks for QueryCache and MutationCache
* test: tests for query onSettled callback
* test: tests for mutation onSettled callback
* docs: onSettled callbacks
* release: v4.26.0
* fix(core): make sure mutations get updated options (#5085)
this fixes an issue around stale closures where callbacks are not updated, thus are called with wrong values in the closure
* release: v4.26.1
* fix(eslint-plugin): improve object property checks (#5079)
* fix(eslint-plugin): improve object property checks
* prettier
* release: v4.26.2
* docs: add adapter dropdown to issue template (#5108)
* docs(queries): rename `success` (#5110)
* chore: `test:lib` task-caching w/ Nx (#5116)
* adding quick caching test
* reverting workflow change
* updating pr yml to test nx speed
* fixing yml on property
* fixing yml on property
* fixing target name
* upping to running4 in parallel
* upping to running 5 in parallel
* upping to running 6 in parallel
* upping to running 7 in parallel
* upping to running 8 in parallel
* upping to running 9 in parallel
* upping to running 10 in parallel
* opting for --parallel=5
* cleaning up nx.json
* revert touching of ci.yml file
* reverting on property of pr.yml file
* updating root pacakge.json test:lib command
* fixing frozen lockfile error in ci
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
* chore: fix missing dependencies (#5127)
* chore: include scripts directory in linting
and fix issues
* chore: add missing dependencies chalk and semver
* chore: turning off Nx daemon in CI (#5128)
* chore: downgrade chalk to v4 because v5 is ESM only (#5130)
see: https://stackoverflow.com/questions/70309135/chalk-error-err-require-esm-require-of-es-module
* fix(eslint-plugin): ignore internal properties (#5119)
* chore: resolve merge conflicts
---------
Co-authored-by: Dominik Dorfmeister <office@dorfmeister.cc>
Co-authored-by: Abhijeet Singh <contact.abhijeetsingh@gmail.com>
Co-authored-by: Tanner Linsley <tannerlinsley@users.noreply.github.com>
Co-authored-by: janinegygax <32389974+janinegygax@users.noreply.github.com>
Co-authored-by: Youssouf Oumar <63708012+yousoumar@users.noreply.github.com>
Co-authored-by: remolueoend <remolueoend@users.noreply.github.com>
Co-authored-by: Eliya Cohen <co.eliya2@gmail.com>
Co-authored-by: Damian Osipiuk <osipiukd+git@gmail.com>
Co-authored-by: Leon Fong <ooohmydawn@hotmail.com>
Co-authored-by: Zachary DeRose <zack@nrwl.io>
Co-authored-by: Zachary DeRose <zack.derose@gmail.com>
Sign up for freeto 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

@ZackDeRose@codecov-commenter@TkDodo@JamesHenry