fix: propagate changed options to a running supercluster worker

[zigzagdev]

Summary

useSuperclusterWorker silently ignored changes to its options argument (radius, maxZoom, minPoints, etc.) after the initial mount, so a running worker kept clustering with whatever options were passed in on the very first render.

Motivation

The worker-init effect only sends the init message to the worker when workerUrl changes, by design — it intentionally excludes options from its dependency array so that unrelated re-renders don't tear down and recreate the worker.
But nothing else ever pushed a changed options value to an already-running worker: optionsRef.current was kept up to date, but that ref was only read once, at worker creation time.
As a result, changing clustering options after mount had no observable effect, even though the ref itself was correct.

What I have done

• Fixed src/hooks/use-supercluster-worker.ts to re-send init (plus load and getClusters, since re-init discards loaded data) to the running worker whenever options changes after mount, instead of only on the initial run.
• Added src/hooks/__tests__/use-supercluster-worker.test.tsx with a MockWorker to verify the worker receives these messages correctly.

Test Plans

• Add a regression test suite asserting the worker receives updated init/load/getClusters messages after an options change.
• Confirm the new tests fail against the pre-fix code and pass against the fix.
• Run the full jest suite to check for regressions.
• Run eslint and tsc --noEmit on the changed files.

Test Results

$ docker run --rm -v $(pwd):/app -w /app node:22-bullseye npx jest

PASS src/hooks/tests/use-supercluster-worker.test.tsx
  ✓ sends the initial options to the worker on mount
  ✓ propagates changed options to an already-running worker
  ✓ reloads previously loaded data after an options change, since re-init discards it
  ✓ requests fresh clusters for the current viewport after an options change
  ✓ does not touch the worker when rerendering with the same options reference
  ... (19 other suites)

Test Suites: 20 passed, 20 total
Tests:       1 skipped, 10 todo, 156 passed, 167 total

`eslint` and `tsc --noEmit` are clean. The new tests were also run against the pre-fix code and failed as expected (no `init`/`load`/`getClusters` messages were sent), confirming they catch the regression.

[samithahansaka]

Thanks for catching this. The bug is real and it is my fault: the options were stored but only ever read when the worker was first created, so later changes never reached it.

One issue with the fix.

The effect depends on [options], which React compares by reference. Most people will call the hook like this:

useSuperclusterWorker(geojson, {radius: 60}, viewport, workerUrl);

That object is new on every render, so React will treat it as changed every time. The worker would be re-initialised, all features reloaded, and clusters re-requested on every render. That would be worse than the original bug.

The example app does not show this because it defines its options outside the component, so the reference stays the same.

This repo already has a fix for this. use-map-options.ts and use-map-3d-options.ts use useDeepCompareEffect from src/hooks/use-deep-compare-effect.ts, which compares values instead of references. Using that here should keep all five of your tests passing.

Also, minor and not blocking: isReadyRef is never read anywhere, so setting it to false has no effect. That is leftover from my original PR, not something you added.

Nice tests. The one showing that re-init throws away loaded data is a good catch.

[usefulthink]

First of all: thanks a lot for the fix and detailed observations!

I am a bit perplexed at the moment.

useSuperclusterWorker() should have never been part of the main library in the first place 😬

It is too specific, introduces external dependencies, and is only tangentially related to the library itself. I'll need to investigate how that happened, but we'll probably have to move it to the examples where it belongs.
But I'm going to do that after applying the bugfix here...

[usefulthink]

Ok, so apparently I missed removing the files in ./src before merging @samithahansaka's PR (#891) in January and never noticed the files since then...

[samithahansaka]

Sorry about that, the duplicate in ./src was my miss in #891.

If it helps, the cleanup is smaller than it looks. It is a deletion rather than a move, because the example never used the library copies:

• src/hooks/use-supercluster-worker.ts
• src/hooks/use-map-viewport.ts

Both already exist under examples/worker-marker-clustering/src/hooks/, and app.tsx imports those local copies. The two src files are not exported from src/index.ts, not imported anywhere inside src, have no tests, and supercluster is not a dependency of the library, so nothing reachable from the published package points at them. The only differences between the copies are a doc link and whether useMap is imported relatively or from @vis.gl/react-google-maps.

Happy to open that PR if you would rather not spend the time on it, or to leave it to you since you are already in there. Either is fine with me.

On the fix itself, no rush on my comment above given the code is moving anyway. The reference equality point still applies once it lives in the examples, but it matters less there, since the example controls its own call site.

[usefulthink]

You're right, this should be fine. I'll apply this fix to the files in the example, and we can just delete the files from ./src. Since those were never actually exported, that shouldn't cause any issues. Otherwise we'd have to worry about breaking backwards compatibility...

Feel free to open a PR deleting the two files if you want. Otherwise, I'll just do it in this PR...

[zigzagdev]

Thanks @samithahansaka for the review and @usefulthink for the context.

This makes sense that this belongs in the examples.

@samithahansaka's point about reference equality is valid. If the fix gets applied to the examples copy, using useDeepCompareEffect (or documenting that options should be memoized) would avoid the re-init-on-every-render issue.

@usefulthink Happy to update this PR to target the examples copy and include the src/ deletion, if that's easier than splitting the work.
Otherwise, feel free to take it from here.

Either way works for me.

Best regards.