Skip to content

Better TypeScript types for viewer-datagrid#3116

Merged
texodus merged 2 commits intomasterfrom
datagrid-types
Feb 4, 2026
Merged

Better TypeScript types for viewer-datagrid#3116
texodus merged 2 commits intomasterfrom
datagrid-types

Conversation

@texodus
Copy link
Member

@texodus texodus commented Feb 4, 2026

This PR updates viewer-datagrid to use new TypeScript types exporte din regular-table@0.8.1, and fixes a flapping test.

@texodus texodus added the internal Internal refactoring and code quality improvement label Feb 4, 2026
Signed-off-by: Andrew Stein <steinlink@gmail.com>
Signed-off-by: Andrew Stein <steinlink@gmail.com>
@texodus texodus marked this pull request as ready for review February 4, 2026 15:59
Copy link
Contributor

@sinistersnare sinistersnare left a comment

Choose a reason for hiding this comment

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

LGTM wrote some nits out but they are very minor.

RegularTable,
DatagridModel,
PerspectiveViewerElement,
import { CellMetadataBody } from "regular-table/dist/esm/types.js";
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be great if this was exported at the top level of regular-table instead of needing to reach into its dist/ dir

Comment on lines -225 to +218
get_psp_type,
// get_psp_type,
Copy link
Contributor

Choose a reason for hiding this comment

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

dead code

// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

import { CellMetadata } from "regular-table/dist/esm/types.js";
import { CellMetadataBody } from "regular-table/dist/esm/types.js";
Copy link
Contributor

Choose a reason for hiding this comment

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

Same nit here with the import (as elsewhere in the code that uses these type imports


const [hex, r, g, b] = colorRecord;

// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

This pattern is used elsewhere, maybe we shoul dkeep the WithFlags bit

Copy link
Member Author

Choose a reason for hiding this comment

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

TypeScript can't extend union types.

@texodus texodus merged commit b5f3732 into master Feb 4, 2026
16 checks passed
@texodus texodus deleted the datagrid-types branch February 4, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Internal refactoring and code quality improvement

Development

Successfully merging this pull request may close these issues.

2 participants