fix/type-hints#243
Merged
TrevorBurgoyne merged 14 commits intomainfrom May 6, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens and validates the TypeScript surface for ulabel npm consumers by updating published typings/exports, making the TS build stricter, and adding automated type-level verification against the packed tarball.
Changes:
- Enable stricter TypeScript checking and adjust source typings/nullability to match runtime behavior.
- Expand/fix
index.d.tsexports and public API type signatures (constructor kwargs, static methods, null/undefined allowances). - Add unit/E2E/type-level tests plus a CI “type-tests” job to compile consumer typings from the packed npm tarball.
Reviewed changes
Copilot reviewed 28 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tsconfig.json |
Enables strict and excludes tests/types from the main TS build. |
tests/ulabel.test.js |
Adds unit tests for keybind/task_meta defaults and replaceLowerConcat behavior. |
tests/types/tsconfig.json |
Adds dedicated TS config for consumer-style type compilation. |
tests/types/index.test-d.ts |
Adds type-level tests validating exported API/types compile for consumers. |
tests/e2e/api-behavior.spec.js |
Adds E2E contract tests around null-dependent API behavior and distances/classification. |
tests/annotation.test.js |
Adds unit test ensuring classification payloads are preserved on resume. |
src/version.js |
Bumps version constant to 0.23.4. |
src/version.d.ts |
Adds declaration for ULABEL_VERSION. |
src/utilities.ts |
Updates get_active_class_id typing to `number |
src/toolbox.ts |
Adjusts typings/nullability, replaces replaceAll with split/join, and adds non-null assertions in DOM/jQuery paths. |
src/toolbox_items/submit_buttons.ts |
Types submit payload and widens annotation variable nullability. |
src/toolbox_items/keybinds.ts |
Adjusts keybind typing/nullability and event handler typing for jQuery namespacing. |
src/toolbox_items/image_filters.ts |
Uses definite assignment assertions for slider handlers. |
src/toolbox_items/annotation_list.ts |
Adjusts edit-candidate construction and non-null assertions for annotation fields. |
src/subtask.ts |
Tightens ULabelSubtask field initialization/types and from_json argument typing. |
src/overlays.ts |
Adds definite assignment assertions and refines nullability in overlay math/drawing. |
src/listeners.ts |
Adds non-null assertions for DOM/jQuery values and widens some handler typing. |
src/initializer.ts |
Adds non-null assertions for image dimensions and canvas contexts. |
src/html_builder.ts |
Improves function typing and adds non-null assertions for DOM accesses. |
src/geometric_utils.ts |
Adds ambient reference, refines return types to include null, and removes some .at() usage. |
src/drawing_utilities.ts |
Fixes index typing for HTML color map access. |
src/configuration.ts |
Adds index signature, widens nullable config fields, and tightens toolbox map typing. |
src/canvas_utils.ts |
Widens subtask_key param to `string |
src/blobs.d.ts |
Adds declarations for bundled SVG/constants module. |
src/annotation.ts |
Refines nullability and error typing; widens from_json return type to include null. |
src/annotation_operators.ts |
Refines nullability defaults and adds casts for dynamic property access. |
src/ambient.d.ts |
Adds ambient module declarations for dependencies lacking shipped types. |
src/actions.ts |
Adds non-null assertions/casts for action fields and update_frame argument typing. |
package.json |
Bumps version, adds files whitelist, and runs tsc --noEmit in lint. |
index.d.ts |
Re-exports core types, fixes several public API signatures, and adds kwargs constructor type. |
CHANGELOG.md |
Adds 0.23.4 release notes. |
.github/workflows/test.yml |
Adds a “type-tests” job that packs + compiles typings in a temp consumer project. |
.github/tasks.md |
Tracks the PR’s checklist/tasks related to typing fixes and tests. |
Comments suppressed due to low confidence (2)
src/utilities.ts:50
get_active_class_idcan fall through without returning a value (it logs an error but returnsundefined), which makes callers easy to break (and some call sites now use non-null assertions). Consider returning a safe fallback (e.g., first class id) or throwing/returning an explicit error object so downstream code doesn’t silently proceed with an undefined class id.
export function get_active_class_id(ulabel: ULabel): number | undefined {
// Grab the current subtask from the ulabel object
const current_subtask_key: string = ulabel.state.current_subtask;
const current_subtask: ULabelSubtask = ulabel.subtasks[current_subtask_key];
src/toolbox_items/keybinds.ts:459
KeybindInfo.keyis typed asstring, but class keybinds can legitimately benull(and the UI logic below already accounts for null/undefined). Usingclass_def.keybind!here suppresses the mismatch rather than fixing it. Recommend wideningKeybindInfo.keytostring | null(and dropping the non-null assertion) so types match runtime behavior.
keybinds.push({
key: class_def.keybind!,
label: class_def.name,
description: `Select class: ${class_def.name}`,
configurable: true,
class_id: class_def.id,
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 33 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
src/annotation_operators.ts:458
- This function currently reports missing DOM elements via
console.error. To match the project’s logging approach (and keep errors visible/structured), please uselog_message(..., LogLevel.ERROR/ WARNING)instead, and avoid directconsole.*calls here.
// Check to make sure each element exists before trying to use
if (show_overlay_checkbox === null) {
console.error("filter_points_distance_from_line could not find show_overlay checkbox object");
return_early = true;
}
if (sliders === null || sliders.length === 0) {
console.error("filter_points_distance_from_line could not find any filter distance slider objects");
return_early = true;
src/toolbox_items/keybinds.ts:460
class_def.keybindis intentionallynullwhen not configured, butKeybindInfo.keyis typed asstringand this code forces non-null withclass_def.keybind!. Prefer updatingKeybindInfo.keytostring | nulland passclass_def.keybindthrough directly to avoid masking nullability and reduce the chance of future code assuming it’s always a string.
if (class_def.id === DELETE_CLASS_ID) continue;
keybinds.push({
key: class_def.keybind!,
label: class_def.name,
description: `Select class: ${class_def.name}`,
configurable: true,
class_id: class_def.id,
});
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix type hints
Description
"files"field topackage.jsonto explicitly control published package contents.PR Checklist
package.jsonhas been bumped since last releasepackage.jsonandsrc/version.jsapi_spec.md)changelog.mdBreaking API Changes
No