Replace `defaultProps` with destructuring in function components #7145

ggdouglas · 2025-01-09T16:09:22Z

Copied from React 18 Upgrade feature branch #7142

Proposed changes:

Replaces defaultProps with destructuring in anticipation of lint errors that will be encountered in React 18.3.0.

Fixes lint errors such as the following:

error: 'defaultProps' is deprecated. Use {@link https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Destructuring_assignment#default_valuedefault values for destructuring assignments instead} (deprecation/deprecation) at src/components/card-list/cardList.tsx:61:10:
         59 |     );
         60 | });
       > 61 | CardList.defaultProps = {
            |          ^
         62 |     bordered: true,
         63 |     compact: false,
         64 | };

Replace `defaultProps` with destructuring in function components

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

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

svc-palantir-github · 2025-01-09T17:28:57Z

Revert defaultProps changes to DateInput3 to fix tests

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

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

gluxon

Thanks! Scrolled through and I believe we've properly copied the defaults over. Only question is around whether we should share default values through an export somehow.

gluxon · 2025-01-10T17:20:38Z

packages/core/src/components/dialog/dialog.tsx

@@ -146,7 +146,7 @@ export class Dialog extends AbstractPureComponent<DialogProps> {
                    <div
                        className={classNames(Classes.DIALOG, className)}
                        role={role}
-                        aria-modal={overlayProps.enforceFocus ?? Overlay2.defaultProps?.enforceFocus}


Would it be useful to have Overlay2 still export its default props if we need to reference them internally?

// Overlay2.tsx export const OVERLAY2_DEFAULT_PROPS = { enforceFocus: true };

I think that makes it easier to see the intent of the code here. Otherwise it's more difficult to know why someone chose to default aria-modal to true.

Yep, can do! I 👍 had considered something like this originally, but wavered at the time of implementing. I think this is a good idea for posterity since these can technically be consumed externally. Having the export set of values to point to will give us a migration path in case there are any other consumers we are unaware of.

gluxon · 2025-01-10T17:23:47Z

packages/datetime/test/components/dateInputTests.tsx

@@ -660,7 +660,7 @@ describe("<DateInput>", () => {
            focusInput(wrapper);
            changeInput(wrapper, "4/77/2016");
            blurInput(wrapper);
-            assert.strictEqual(wrapper.find(InputGroup).prop("value"), DateInput.defaultProps?.invalidDateMessage);
+            assert.strictEqual(wrapper.find(InputGroup).prop("value"), "Invalid date");


Similar comment around potentially exporting default props from DateInput.

gluxon · 2025-01-10T17:24:18Z

packages/datetime2/src/components/date-input3/dateInput3.tsx

+// TODO: Removing `defaultProps` here breaks tests. Investigate why.
+// eslint-disable-next-line deprecation/deprecation


That's fun. 😂

Thanks for leaving a comment so others have context on this in the future.

Yeah.. 🥲

I am working on updating/refactoring a lot of the datetime2 tests currently, so will hopefully uncover the cause of this during the process.

svc-palantir-github · 2025-01-10T19:35:32Z

Leverage inferred type for backdropProps to fix type error

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

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

This test passes locally but fails in CI. Need to investigate why this occurs. ``` Chrome Headless 124.0.6367.201 (Linux x86_64) <Popover> focus management when shouldReturnFocusOnClose={true} moves focus to overlay when opened FAILED Error: Uncaught focus should be inside overlay: expected null to not equal null ... ```

svc-palantir-github · 2025-01-10T20:36:57Z

Revert default assignments in Overlay2 props to avoid test flake in CI

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

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

packages/select/src/components/suggest/suggest.tsx

Invalidated by push of aefacb1

svc-palantir-github · 2025-01-11T01:57:05Z

Revert change to replace Popover default props reference in Suggest

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

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

Replace defaultProps with destructuring in function components

b837f58

ggdouglas mentioned this pull request Jan 9, 2025

React 18 Upgrade #7142

Merged

15 tasks

palantir deleted a comment from changelog-app bot Jan 9, 2025

Revert defaultProps changes to DateInput3 to fix tests

5d9ddb9

gluxon reviewed Jan 10, 2025

View reviewed changes

ggdouglas added 4 commits January 10, 2025 13:48

Expose shared export for Overlay2 default props

5084a87

Expose shared export for DateInput(3) props

d491cf1

Keep Overlay2 default props for posterity

eb5d453

Leverage inferred type for backdropProps to fix type error

3a81924

ggdouglas requested a review from gluxon January 10, 2025 19:21

gluxon previously approved these changes Jan 10, 2025

View reviewed changes

packages/select/src/components/suggest/suggest.tsx Outdated Show resolved Hide resolved

Revert change to replace Popover default props reference in Suggest

aefacb1

gluxon approved these changes Jan 11, 2025

View reviewed changes

ggdouglas merged commit 0a63583 into develop Jan 12, 2025
12 of 17 checks passed

ggdouglas deleted the feat/react-18-deprecated-defaultProps branch January 12, 2025 02:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace `defaultProps` with destructuring in function components #7145

Replace `defaultProps` with destructuring in function components #7145

ggdouglas commented Jan 9, 2025 •

edited

Loading

svc-palantir-github commented Jan 9, 2025

svc-palantir-github commented Jan 9, 2025

gluxon left a comment

gluxon Jan 10, 2025

ggdouglas Jan 10, 2025

ggdouglas Jan 10, 2025

gluxon Jan 10, 2025

ggdouglas Jan 10, 2025

gluxon Jan 10, 2025

ggdouglas Jan 10, 2025

svc-palantir-github commented Jan 10, 2025

svc-palantir-github commented Jan 10, 2025

svc-palantir-github commented Jan 11, 2025

		// TODO: Removing `defaultProps` here breaks tests. Investigate why.
		// eslint-disable-next-line deprecation/deprecation

Replace defaultProps with destructuring in function components #7145

Replace defaultProps with destructuring in function components #7145

Conversation

ggdouglas commented Jan 9, 2025 • edited Loading

Proposed changes:

See also:

svc-palantir-github commented Jan 9, 2025

Replace `defaultProps` with destructuring in function components

svc-palantir-github commented Jan 9, 2025

Revert defaultProps changes to DateInput3 to fix tests

gluxon left a comment

Choose a reason for hiding this comment

gluxon Jan 10, 2025

Choose a reason for hiding this comment

ggdouglas Jan 10, 2025

Choose a reason for hiding this comment

ggdouglas Jan 10, 2025

Choose a reason for hiding this comment

gluxon Jan 10, 2025

Choose a reason for hiding this comment

ggdouglas Jan 10, 2025

Choose a reason for hiding this comment

gluxon Jan 10, 2025

Choose a reason for hiding this comment

ggdouglas Jan 10, 2025

Choose a reason for hiding this comment

svc-palantir-github commented Jan 10, 2025

Leverage inferred type for backdropProps to fix type error

svc-palantir-github commented Jan 10, 2025

Revert default assignments in Overlay2 props to avoid test flake in CI

svc-palantir-github commented Jan 11, 2025

Revert change to replace Popover default props reference in Suggest

Replace `defaultProps` with destructuring in function components #7145

Replace `defaultProps` with destructuring in function components #7145

ggdouglas commented Jan 9, 2025 •

edited

Loading