Weren't used at all

This ends up being more important for the upcoming PR, but I really, really think that we need to find the time to formalize our entire data layer, and be way more deliberate with our React Query caching

I fully expect our caching to create a super, super hard-to-recreate bug sometime in the future, because it's a bunch of semi-structured global mutable state right now

Also added an explicitstring type annotation so that TypeScript doesn't over-infer the value and treat it as a literal. We shouldn't know what exact combination of characters it is – just what purpose the string serves in the greater codebase as an opaque value

Decoupled type signature from React Router. Splitting this up gives us a lot more freedom, which comes up later in this PR

I feel like the previous type was super, super risky, because it was derived from a function that didn't have much type-safety

With this, the contract is way more explicit and simple, and there's basically no chance of the TypeScript LSP getting confused and dropping properties for tooltips

We were missing a return type altogether before

Another render optimization

What is this render optimization? Not re-rendering the page if it's the same page?

So there's two parts:

• The basic input validation from theNumber static methods
• The equality check

I could maybe add a comment, but React only hasObject.is for change detection, so in general, a lot of render optimizations boil down to "only dispatch a new value (especially a non-primitive value) when you know the contents have changed". Most of the time, you don't have to worry about over-dispatching primitive values (as long as they're done outside a render, React will detect that nothing changed and do a no-op for the state update). But because JS is wacky, there are edge cases withObject.is and numbers, which is why there's the extra=== check

The extra wrapper function was redundant

Removed type assertion to remove risk of accidentally silencing the compiler

This is part of a fix for a super nasty bug that I ran into when updating the batch-update UI

Because we were storing an entire copy of the original workspaces each time we checked off something, that also meant that as the query cache updated its data and triggered re-renders, anything that used the checked workspaces would receive the stale versions of the workspaces

Slightly re-jiggeredconfirmingBatchAction

Another render optimization

This change becomes a little more relevant for the other PR, but even if this were the only PR, this would still help the update UI avoid stale state

Added explicit return type, which was super important here, because the function's return type was getting inferred as something super complicated and nasty from React Query

Decouples the main state from the actual component props

The way I think about it is that props are a hack to get JS function to support named arguments and make it mirror HTML more, so you really shouldn't ever have any direct dependencies on that

What is this render optimization? Not re-rendering the page if it's the same page?

nit: there seems to be inconsistencies within the code base between usingcamelCase orUPPER_SNAKE_CASE for file-wide constants so while out of scope for this PR I think deciding on one and sticking to it would be good

Yeah, that's fair. The JS/TS way seems to be SCREAMING_CASE for constants, while the Go way seems to be to treat them as any other package value

I'm leaning towards switching this to SCREAMING_CASE, but I'll wait for Kayla to chime in

This becomes exported in the second PR, which is why I put more of a public-facing comment on it

Forgot to comment on this, but theresetChecked logic above is meant to make sure that if we add more URL logic in the future, it doesn't break our filtering and pagination

We were coarsely syncing based on the entire URL params object. That worked fine for now, but if we were to add any logic on the same page that also dealt with URLs and had nothing to do with the filtering/pagination, it would still cause all the checkboxes to get unchecked on every change

Moving the state syncing to the event handlers for the filter/pagination hooks seemed like the better choice. That way, we're also not coupled to the hooks formatting the URLs a specific way either