query icon indicating copy to clipboard operation
query copied to clipboard

**perf** improve perf, as discussed in #8091

Open joeljeske opened this issue 1 year ago • 3 comments

joeljeske avatar Sep 23 '24 21:09 joeljeske

☁️ Nx Cloud Report

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

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx run-many --target=build --exclude=examples/** --exclude=integrations/**
✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

nx-cloud[bot] avatar Oct 01 '24 09:10 nx-cloud[bot]

Thanks - I think proxies should work well. Just for context, when we started out, we had to support browsers that didn’t have proxy support and we never re-visited it.

I think there are two other places that use Object.defineProperty. Can you take a stab at them as well please?

also, docs would need an update here please:

https://tanstack.com/query/v5/docs/framework/react/guides/render-optimizations#tracked-properties

@TkDodo Thanks for this! @joeljeske and I work together and from our profiling this is the only major hot spot. We didn't see improvement in updating the other call sites. Does your test suite cover any performance tests to assert no regressions if other sites are updated? I think it may be useful to incrementally land this as this one is a known issue that we discovered.

Also happy to help update the docs, what would you like updated? I can help take this over from @joeljeske as we'd love to give this fix back to the community and not maintain it internally

Aghassi avatar Oct 03 '24 17:10 Aghassi

We didn't see improvement in updating the other call sites

Understood, but for having a unified approach, I think we should use either proxies or Object.defineProperty everywhere.

Also happy to help update the docs, what would you like updated?

This page (https://tanstack.com/query/v5/docs/framework/react/guides/render-optimizations#tracked-properties) shouldn’t refer to custom getters anymore, but state that Proxy is used instead.

TkDodo avatar Oct 09 '24 14:10 TkDodo

@joeljeske @Aghassi do you plan to continue working on this contribution ?

TkDodo avatar Nov 15 '24 15:11 TkDodo

@TkDodo yes sorry, been very busy with work and the holidays. I will be taking this up, sorry for the delay

Aghassi avatar Dec 03 '24 16:12 Aghassi

2 more months have passed, @Aghassi can we please wrap this up, otherwise I’ll close the PR. Thanks 🙏

TkDodo avatar Feb 16 '25 15:02 TkDodo

superseded by:

  • https://github.com/TanStack/query/pull/9079

TkDodo avatar Apr 29 '25 09:04 TkDodo