Skip to content

chore(deps): adapt to typescript 6 and strict mode#1462

Draft
CommanderStorm wants to merge 17 commits into
maplibre:mainfrom
CommanderStorm:ts-update-typescript6
Draft

chore(deps): adapt to typescript 6 and strict mode#1462
CommanderStorm wants to merge 17 commits into
maplibre:mainfrom
CommanderStorm:ts-update-typescript6

Conversation

@CommanderStorm

Copy link
Copy Markdown
Member

I asked Claude to update us to ts6, which also apparently means strict mode.

A few of the changes that were nessary seem not quite right, but I need to investigate further.

PR mostly to give notice that I am working on this

@CommanderStorm CommanderStorm changed the title chore(deps): adapt to typescript 6 chore(deps): adapt to typescript 6 and strict mdoe Jun 23, 2026
@CommanderStorm CommanderStorm changed the title chore(deps): adapt to typescript 6 and strict mdoe chore(deps): adapt to typescript 6 and strict mode Jun 23, 2026
@codecov-commenter

codecov-commenter commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.25287% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.39%. Comparing base (ec6808d) to head (eb718a5).

Files with missing lines Patch % Lines
ts/src/decoding/decodingUtils.ts 50.00% 1 Missing ⚠️
ts/src/decoding/geometryDecoder.ts 90.00% 1 Missing ⚠️
ts/src/decoding/integerStreamDecoder.ts 50.00% 0 Missing and 1 partial ⚠️
ts/src/encoding/integerStreamEncoder.ts 50.00% 0 Missing and 1 partial ⚠️
ts/src/mltDecoder.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1462      +/-   ##
==========================================
+ Coverage   96.04%   96.39%   +0.34%     
==========================================
  Files          68       68              
  Lines        4880     4909      +29     
  Branches      764      787      +23     
==========================================
+ Hits         4687     4732      +45     
+ Misses        178      163      -15     
+ Partials       15       14       -1     

☔ 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.

symbolLengthStream: Uint32Array | null,
nullabilityBuffer: BitVector | null,
): Vector | null {
symbolTableStream: Uint8Array | undefined,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not sure if undefined vs null is better here.

I just know that this being uneven between calle and caller makes for lots of akward conversion casts..

@CommanderStorm
CommanderStorm marked this pull request as ready for review June 23, 2026 22:16
Copilot AI review requested due to automatic review settings June 23, 2026 22:16

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.

@CommanderStorm
CommanderStorm requested a review from HarelM June 23, 2026 22:21
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@HarelM

HarelM commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

There's too many "as something" added, which I find odd like you did I think... It would be better to avoid those if possible.

Comment thread ts/src/encoding/fastPforEncoder.ts Outdated
Comment thread ts/src/metadata/tileset/embeddedTilesetMetadataDecoder.ts Outdated
Comment thread ts/src/metadata/tileset/embeddedTilesetMetadataDecoder.ts Outdated
CommanderStorm and others added 3 commits June 24, 2026 11:36
decodeString legitimately returns undefined for columns with no
recognized string encoding (e.g. all zero-length streams); the
previous `as Vector` cast masked this. Type it `Vector | undefined`
instead of throwing, and map to null at the property-decoder call
site (its existing "absent column" signal).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CommanderStorm

Copy link
Copy Markdown
Member Author

There's too many "as something" added, which I find odd like you did I think... It would be better to avoid those if possible.

a lot more change, but sure. I updated the code to abort with better error messages on invariant violation

CommanderStorm and others added 4 commits June 24, 2026 11:48
geometryVector starts as null and `as GeometryVector | GpuVector` just
stripped it. A feature table without a geometry column is malformed, so
throw a descriptive error rather than constructing a FeatureTable with a
null geometry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CommanderStorm

Copy link
Copy Markdown
Member Author

There's too many "as something" added, which I find odd like you did I think... It would be better to avoid those if possible.

Removed them, but that forced a lot of code churn, added aborts and other things.

@HarelM

HarelM commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

This PR is a lot to review, any chance to slice it to smaller PRs?

@CommanderStorm
CommanderStorm marked this pull request as draft June 24, 2026 12:29
@CommanderStorm

Copy link
Copy Markdown
Member Author

needs breaking apart. a bit too non-atomic.

CommanderStorm added a commit that referenced this pull request Jul 1, 2026
Split from #1462:
build Column/Field as immutable discriminated-union literals (adds
ColumnWithoutName) instead of `{} as Column` plus mutation.
@HarelM

HarelM commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

While I don't fully stand behind this statement, I've checked locally and you can set strcit: false to update to typescript 6.
There's also a way to add more granular stricktiness check and avoid the strict: true as I find it a bit incorrect in some places and adds code just to satisfy the strict check without meaningful value.
At least that's my experience with it.
So I think a smaller change can be made here, but still leverage some valuable improvements strict is showing us.

CommanderStorm added a commit that referenced this pull request Jul 23, 2026
Split from #1462.

- `decodeString` now returns `undefined` for empty columns, and the
dictionary/FSST decoders throw a named error when a column's companion
streams are incomplete instead of constructing a half-built vector.
- `propertyDecoder` keeps its existing `null` contract via `?? null`.

Adds coverage for the empty/incomplete cases
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