Skip to content

fix(ts): Add defensive guards on geometry decode paths#1470

Open
CommanderStorm wants to merge 5 commits into
maplibre:mainfrom
CommanderStorm:ts-geometry-decode-guards
Open

fix(ts): Add defensive guards on geometry decode paths#1470
CommanderStorm wants to merge 5 commits into
maplibre:mainfrom
CommanderStorm:ts-geometry-decode-guards

Conversation

@CommanderStorm

@CommanderStorm CommanderStorm commented Jun 24, 2026

Copy link
Copy Markdown
Member

Split out from #1462

Adds named-error guards on the geometry decode paths.
Missing vertex/triangle/offset buffers/ morton settings fail with a clear message instead of a later errors.

Copilot AI review requested due to automatic review settings June 24, 2026 12:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@codecov-commenter

codecov-commenter commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.43590% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.14%. Comparing base (5c0ae60) to head (a508ab5).
⚠️ Report is 86 commits behind head on main.

Files with missing lines Patch % Lines
ts/src/decoding/geometryDecoder.ts 90.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1470       +/-   ##
===========================================
+ Coverage   63.96%   96.14%   +32.18%     
===========================================
  Files          88       68       -20     
  Lines       14440     4935     -9505     
  Branches        0      779      +779     
===========================================
- Hits         9236     4745     -4491     
+ Misses       5204      175     -5029     
- Partials        0       15       +15     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@CommanderStorm CommanderStorm changed the title Add defensive guards on geometry decode paths fix(ts): Add defensive guards on geometry decode paths Jun 24, 2026
@CommanderStorm
CommanderStorm requested a review from HarelM June 30, 2026 09:00
Comment thread ts/src/vector/geometry/gpuVector.ts Outdated
@HarelM

HarelM commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Are these possible code flows? I'm not very keen on defensive programing...

Comment thread ts/src/decoding/geometryDecoder.ts
Comment thread ts/src/vector/geometry/geometryVectorConverter.ts Outdated
@HarelM

HarelM commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Looking at the errors that were added, I'm not sure they are a lot better than "x is not a property of undefined", as they don't really help understanding where the issue actually is and how to solve it.
IDK...

Removed the iterator method from GpuVector class, which was not implemented and contained commented-out code.
Removed unimplemented GpuVector iterator tests.
@CommanderStorm

Copy link
Copy Markdown
Member Author

The issues should not be producible on valid input by definition.
So if we design our encoders well, they are impossible by construction.

Problem is that I still will have undefined in the type, and that is not something I think I can fix..

@CommanderStorm

Copy link
Copy Markdown
Member Author

If we want ts6, strict mode seems to be one of the requirements, so therefore we need to actually handle our undefined..

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.

4 participants