Skip to content

Add Etag support to the coming vector/geojson worker refactor.#7074

Open
wayofthefuture wants to merge 20 commits intomainfrom
geojson-worker-abstract-etag
Open

Add Etag support to the coming vector/geojson worker refactor.#7074
wayofthefuture wants to merge 20 commits intomainfrom
geojson-worker-abstract-etag

Conversation

@wayofthefuture
Copy link
Collaborator

@wayofthefuture wayofthefuture commented Feb 7, 2026

This is a PR against the current PR #7058 that creates more separation between vector and geojson workers. Putting up this draft PR so we can visualize the differences (and show patch coverage) between the new refactored workers and the coming etag support, which is modifying many of the same files. Feel free to comment. Am trying to figure out the best way to abstract the vector/geojson workers from each other while maintaining the coming etag support in a clean way.

The etag support in this PR tries to leverage the existing workertile result pipeline, and adds a LoadTileResult type to loadTile - which carries an etagUnmodified property. I figured this result type would be beneficial as it allows further properties to be added in the future to loadTile. In order to separate downloading from tile creation (for etag), the network request and creation of the vector tile data are separated so that when the etag hasn't changed the processing does not occur. Coincidentally now both loadVectorTile's in both workers are synchronous and separate tile processing from any network requests.

@codecov
Copy link

codecov bot commented Feb 7, 2026

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.58%. Comparing base (b3990b5) to head (dcd5b5b).

Files with missing lines Patch % Lines
src/source/vector_tile_worker_source.ts 94.73% 1 Missing ⚠️
src/tile/tile_manager.ts 80.00% 1 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           geojson-worker-abstract    #7074   +/-   ##
========================================================
  Coverage                    92.57%   92.58%           
========================================================
  Files                          289      289           
  Lines                        23987    24002   +15     
  Branches                      5083     5088    +5     
========================================================
+ Hits                         22206    22222   +16     
+ Misses                        1781     1780    -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@wayofthefuture wayofthefuture marked this pull request as ready for review February 7, 2026 16:38
Base automatically changed from geojson-worker-abstract to main February 8, 2026 06:58
@HarelM
Copy link
Collaborator

HarelM commented Feb 8, 2026

@wayofthefuture I'll let you handle this merge, I tried it myself but was not 100% sure...

Sign up for free to 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.

2 participants