Use generics for Select arrays #7036

ggdouglas · 2024-10-30T19:48:42Z

Fixes #0000

Checklist

Includes tests
Update documentation

Changes proposed in this pull request:

This change is an alternative approach to #6999, allowing arrays passed to Select (as well as other components that take arrays) to be marked as readonly. It also preserves backwards compatibility with previous types such that the new prop types will not break existing consumers of these components where they are already passing mutable arrays. This will make it easier to transition existing implementations to use readonly arrays.

Reviewers should focus on:

Screenshot

changelog-app · 2024-10-30T19:48:47Z

Generate changelog in `packages/docs-app/changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

Use generics for Select arrays

Generate changelog in `packages/select/changelog/@unreleased`

What do the change types mean?

feature: A new feature of the service.
improvement: An incremental improvement in the functionality or operation of the service.
fix: Remedies the incorrect behaviour of a component of the service in a backwards-compatible way.
break: Has the potential to break consumers of this service's API, inclusive of both Palantir services
and external consumers of the service's API (e.g. customer-written software or integrations).
deprecation: Advertises the intention to remove service functionality without any change to the
operation of the service itself.
manualTask: Requires the possibility of manual intervention (running a script, eyeballing configuration,
performing database surgery, ...) at the time of upgrade for it to succeed.
migration: A fully automatic upgrade migration task with no engineer input required.

Note: only one type should be chosen.

How are new versions calculated?

❗The break and manual task changelog types will result in a major release!
🐛 The fix changelog type will result in a minor release in most cases, and a patch release version for patch branches. This behaviour is configurable in autorelease.
✨ All others will result in a minor version release.

Type

Description

Use generics for Select arrays

Check the box to generate changelog(s)

Generate changelog entry

ggdouglas · 2024-10-30T20:12:11Z

packages/docs-app/src/examples/select-examples/multiSelectExample.tsx

@@ -40,13 +40,13 @@ const INTENTS = [Intent.NONE, Intent.PRIMARY, Intent.SUCCESS, Intent.DANGER, Int

 export interface MultiSelectExampleState {
    allowCreate: boolean;
-    createdItems: Film[];
+    createdItems: readonly Film[];


The modification of the examples here in this PR isn't strictly necessary, though it helps to serve as a practical example of how to migrate existing Select/MultiSelect components to use readonly arrays. It's also worth noting that the examples here still compile fine if they are not switched to use readonly arrays, demonstrating the backwards compatibility of these type changes.

All the docs/test changes are contained within: 19cdbee

svc-palantir-github · 2024-10-30T20:13:17Z

Update examples and tests to use readonly array

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

ggdouglas · 2024-10-30T20:28:33Z

packages/docs-app/src/examples/select-examples/multiSelectExample.tsx

@@ -124,7 +124,7 @@ export class MultiSelectExample extends React.PureComponent<ExampleProps, MultiS

        return (
            <Example options={this.renderOptions()} {...this.props}>
-                <MultiSelect<Film>


The omission of the specified Film generic parameter to the component is necessary here, otherwise we'll get a type error:

'readonly Film[]' is 'readonly' and cannot be assigned to the mutable type 'Film[]'

Another way to solve this is to explicitly define the second type parameter, e.g.

<MultiSelect<Film, readonly Film[]>

svc-palantir-github · 2024-10-30T20:42:02Z

Update examples and tests to use readonly array

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

invliD

Nice!

invliD · 2024-10-31T09:09:00Z

packages/docs-app/src/examples/select-examples/multiSelectExample.tsx

-                nextItems = results.items;
-                nextCreatedItems = results.createdItems;
+                nextItems = results.items.slice();
+                nextCreatedItems = results.createdItems.slice();


Do we have to slice these? We're not actually mutating either of them below.

Ah, yeah, good call. I originally added the slice here since, otherwise, we'd be assigning the readonly arrays from results (results.items and results.createdItems) to mutable variables, which results in a type error. We can remove the slice altogether and make everything consistently readonly. 5b8c385

Invalidated by push of 5b8c385

svc-palantir-github · 2024-10-31T15:39:47Z

Fix typo in Suggest tests

Build artifact links for this commit: documentation | landing | table | demo

This is an automated comment from the deploy-preview CircleCI job.

This reverts commit 4d1297a.

Use generic for specifying select item mutability

8f5925a

ggdouglas force-pushed the gdouglas/generic-readonly-select branch from 21585a8 to af3c19a Compare October 30, 2024 19:58

ggdouglas commented Oct 30, 2024

View reviewed changes

ggdouglas requested review from invliD and cybae0804 October 30, 2024 20:16

Update examples and tests to use readonly array

19cdbee

ggdouglas force-pushed the gdouglas/generic-readonly-select branch from af3c19a to 19cdbee Compare October 30, 2024 20:27

ggdouglas commented Oct 30, 2024

View reviewed changes

invliD previously approved these changes Oct 31, 2024

View reviewed changes

jscheiny previously approved these changes Oct 31, 2024

View reviewed changes

Remove slice around readonly arrays

5b8c385

Fix typo in Suggest tests

25f1aa5

ggdouglas force-pushed the gdouglas/generic-readonly-select branch from b72900e to 25f1aa5 Compare October 31, 2024 15:25

invliD approved these changes Oct 31, 2024

View reviewed changes

ggdouglas merged commit 4d1297a into develop Oct 31, 2024
12 of 15 checks passed

ggdouglas deleted the gdouglas/generic-readonly-select branch October 31, 2024 18:10

jscheiny added a commit that referenced this pull request Nov 5, 2024

Revert "Use generics for Select arrays (#7036)"

5de0a28

This reverts commit 4d1297a.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use generics for Select arrays #7036

Use generics for Select arrays #7036

ggdouglas commented Oct 30, 2024 •

edited

Loading

changelog-app bot commented Oct 30, 2024

ggdouglas Oct 30, 2024 •

edited

Loading

svc-palantir-github commented Oct 30, 2024

ggdouglas Oct 30, 2024

svc-palantir-github commented Oct 30, 2024

invliD left a comment

invliD Oct 31, 2024

ggdouglas Oct 31, 2024

svc-palantir-github commented Oct 31, 2024

Use generics for Select arrays #7036

Use generics for Select arrays #7036

Conversation

ggdouglas commented Oct 30, 2024 • edited Loading

Fixes #0000

Checklist

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

changelog-app bot commented Oct 30, 2024

Generate changelog in packages/docs-app/changelog/@unreleased

Generate changelog in packages/select/changelog/@unreleased

ggdouglas Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

svc-palantir-github commented Oct 30, 2024

Update examples and tests to use readonly array

ggdouglas Oct 30, 2024

Choose a reason for hiding this comment

svc-palantir-github commented Oct 30, 2024

Update examples and tests to use readonly array

invliD left a comment

Choose a reason for hiding this comment

invliD Oct 31, 2024

Choose a reason for hiding this comment

ggdouglas Oct 31, 2024

Choose a reason for hiding this comment

svc-palantir-github commented Oct 31, 2024

Fix typo in Suggest tests

ggdouglas commented Oct 30, 2024 •

edited

Loading

Generate changelog in `packages/docs-app/changelog/@unreleased`

Generate changelog in `packages/select/changelog/@unreleased`

ggdouglas Oct 30, 2024 •

edited

Loading