Skip to content

Stricter typing across the library - #77

Open
Karinon wants to merge 7 commits into
mainfrom
stricter-types
Open

Karinon wants to merge 7 commits into
mainfrom
stricter-types

Conversation

@Karinon

@Karinon Karinon commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator

Mostly a typing cleanup. Behaviour should stay the same except for the fixes listed below. This is the first iteration of stricter types. The goal is to replace all occurrences of any in the end. This is not really a breaking change, but one could argue it is, as some of the types are now enforced.

Changes

  • Dependencies: updated dev dependencies (cpx2, rimraf) and added @types/emscripten. The hand-written any types for the Emscripten module and FS are replaced with the official types.
  • Error handling: new toError() helper in errors.ts, with tests. All catch (err: any) blocks now use unknown plus toError().
  • Shared types: AnyTypedArray moved into types.ts and replaces the long typed-array unions it duplicated.
  • Stricter constants and modes:
    • NC_CONSTANTS is now as const.
    • New FILE_MODES table, with derived FileMode / OpenMode / CreateMode types.
    • mode in Dataset() and the from* factories, including the public wrappers in index.ts, is now FileMode instead of string.
    • Worker messages are typed.
  • Cleanup: removed duplicate NC_LONGLONG / NC_ULONGLONG entries from the getter maps. They are aliases of NC_INT64 / NC_UINT64 and only became visible as errors after as const.

Fixes found along the way

  • Attribute and group-name lengths read as 64-bit values are now converted to number. Emscripten returns them as BigInt, which the old types hid.
  • open() handles the worker path and the regular read path in one branch. Creating files works as before.

Notes

  • TYPE_NAMES is commented out, since nothing used it.

@lazarusA

lazarusA commented Nov 4, 2026

Copy link
Copy Markdown
Member

pretty much in favour of deleting code (but some could remain as placeholders in the meantime, only if, needed and planned).

Is this good to go?

Comment thread src/constants.ts
15: "enum",
16: "compound",
};
// export const TYPE_NAMES: { [key: number]: string } = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think I wanna to use these somewhere else, but forgot, plus knowing what each meant was useful for debugging. So, maybe delete? if they are really not used.

Comment thread src/types.ts
Comment on lines -31 to -40
HEAPF64: Float64Array;
HEAP64: BigInt64Array;
HEAP32: Int32Array;
HEAP8: Int8Array;
HEAPF32: Float32Array;
HEAPU8: Uint8Array;
HEAP16: Int16Array;
HEAPU16: Uint16Array;
HEAPU32: Uint32Array;
HEAPU64: BigUint64Array;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

where are these now?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Surprised it passed Linting without these.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

They are not (entirely) gone. They are part of the new dependency "@types/emscripten". Hence the

export interface EmscriptenModule extends globalThis.EmscriptenModule {

which is also the reason why the linting worked.

I thought it might make sense to reuse something already existing and just to extend it with the bits we need.

I am going to respond slower this week due to events in my company.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Was feeling dumb cause this didn't make sense. But the global EmscriptenModule is declared in node_modules,. So this is solid

This branch has not been deployed

No deployments
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.

3 participants