Skip to content

MM-69835 Update usage of propTypes and defaultProps - #12

Merged
hmhealey merged 5 commits into
MM-69835-3from
MM-69835-4
Aug 11, 2026
Merged

MM-69835 Update usage of propTypes and defaultProps#12
hmhealey merged 5 commits into
MM-69835-3from
MM-69835-4

Conversation

@hmhealey

@hmhealey hmhealey commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

React 19 makes two changes to those:

  1. propTypes is no longer checked at runtime, so some tests that relied on the warnings printed by that have been replaced with regular runtime checks using warning which only function when the NODE_ENV isn't production. Some custom validators had to be rewritten for this.
  2. Function components can no longer have defaultProps.

The source code for this adds a lot of those by using some helpers like bsSize which had to be rewritten to adapt to that.

Ticket Link

https://mattermost.atlassian.net/browse/MM-69835

@hmhealey hmhealey changed the title Mm 69835 4 MM-69835 Update usage of propTypes and defaultProps Aug 4, 2026
Comment thread src/utils/bootstrapUtils.js Outdated
// render, without wrapping the component (which would break ref forwarding,
// static reads, and — for e.g. `bsRole` — class `defaultProps` merging).
function patchRenderValidation(Component, propName, allowed) {
const flag = `__bsValidated_${propName}`;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, this is ugly, but I couldn't come up with a better solution than what was generated by Claude without going through each component and manually adding the propTypes and default values

@hmhealey

hmhealey commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@coderabbitai Review this

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

@hmhealey I will review pull request #12, with focus on React 19 compatibility for propTypes, defaultProps, and runtime warnings.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Aug 4, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The change moves several prop validations from declaration time to render time. It adds shared role-error helpers, updates component warnings, changes bootstrap decorators to validate during rendering, updates tests, and makes generated-file patterns recursive.

Changes

Runtime validation warnings

Layer / File(s) Summary
Role error helpers
src/utils/PropTypes.js
Role checks now return missing or duplicate role messages instead of chainable PropTypes errors.
Component render-time warnings
src/ButtonGroup.js, src/Dropdown.js, src/MenuItem.js, src/Nav.js, src/ProgressBar.js, test/DropdownSpec.js, test/ProgressBarSpec.js
Components use basic prop declarations and report invalid prop combinations or children during rendering.
Bootstrap decorator handling
src/utils/bootstrapUtils.js, test/utils/bootstrapUtilsSpec.js, test/index.js
Bootstrap decorators apply defaults and validate allowed values during rendering for function and class components. Tests now render decorated components and treat default-prop warnings as errors.

Generated file patterns

Layer / File(s) Summary
Recursive generated-file patterns
.gitattributes
The es and lib patterns now match nested files. The dist pattern is unchanged.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Dropdown
  participant RoleErrorHelpers
  participant warning
  Dropdown->>RoleErrorHelpers: check required and duplicate roles
  RoleErrorHelpers-->>Dropdown: return warning message or null
  Dropdown->>warning: report invalid role configuration
Loading
sequenceDiagram
  participant DecoratedComponent
  participant applyBsProp
  participant React
  participant warning
  DecoratedComponent->>applyBsProp: apply defaults and validate props
  applyBsProp->>React: create wrapped element
  applyBsProp->>warning: report invalid allowed values
  React-->>DecoratedComponent: render element
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: updating propTypes and defaultProps usage for React 19 compatibility.
Description check ✅ Passed The description directly explains the React 19 compatibility changes to propTypes, defaultProps, validators, and runtime warnings.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch MM-69835-4

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/ProgressBar.js (1)

21-43: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Guard empty children before reading child.type. React.Children.forEach invokes its callback for null, undefined, and booleans. When a truthy children array contains one of these values, Line 36 throws before warning runs. Skip empty nodes, check React.isValidElement(child) before reading type, and use String(child.type) for unsupported element types. Preserve the existing element.type comparison to retain proxy compatibility.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/ProgressBar.js` around lines 21 - 43, Update getInvalidChildError to skip
null, undefined, and boolean children before accessing child.type, and only read
type after confirming React.isValidElement(child). Preserve the existing
element.type comparison for proxy compatibility, and stringify unsupported
element types with String(child.type) when constructing the error message.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/Nav.js`:
- Around line 283-287: The warning condition in Nav should use the effective
navbar value, including the navbar state derived from navbarContext when
propsNavbar is absent, rather than checking only propsNavbar. Update the
condition near warning() and align it with the value used by the
navbar-rendering logic so justified Nav components in navbar context trigger the
unsupported-style warning.

In `@src/utils/bootstrapUtils.js`:
- Around line 49-60: Update the validator setup around validatedRender so its
mutable configuration is stored under flag rather than captured permanently in
the first allowed array. When the existing flag is found, refresh that
configuration’s allowed value before returning, so later bsSizes calls validate
against rebuilt values; add coverage using two bsSizes decorators on a class
component.
- Around line 99-118: Update curry to detect React.memo and React.forwardRef
results as supported component types instead of treating them as plain objects.
Apply bsClass, bsStyles, bsSizes, default props, and validation while preserving
each component’s memoization or ref behavior. Add React 19 coverage for all
three decorators on both exotic component types.

---

Outside diff comments:
In `@src/ProgressBar.js`:
- Around line 21-43: Update getInvalidChildError to skip null, undefined, and
boolean children before accessing child.type, and only read type after
confirming React.isValidElement(child). Preserve the existing element.type
comparison for proxy compatibility, and stringify unsupported element types with
String(child.type) when constructing the error message.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8b0cd44-f0c8-4776-b284-91acae4f62ec

📥 Commits

Reviewing files that changed from the base of the PR and between 2dd3aae and a182b78.

📒 Files selected for processing (12)
  • .gitattributes
  • src/ButtonGroup.js
  • src/Dropdown.js
  • src/MenuItem.js
  • src/Nav.js
  • src/ProgressBar.js
  • src/utils/PropTypes.js
  • src/utils/bootstrapUtils.js
  • test/DropdownSpec.js
  • test/ProgressBarSpec.js
  • test/index.js
  • test/utils/bootstrapUtilsSpec.js
💤 Files with no reviewable changes (1)
  • test/index.js

Comment thread src/Nav.js
Comment thread src/utils/bootstrapUtils.js Outdated
Comment thread src/utils/bootstrapUtils.js Outdated
@hmhealey
hmhealey requested a review from larkox August 6, 2026 17:27

@larkox larkox left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, just one thing to verify.

Comment thread src/utils/bootstrapUtils.js
@hmhealey
hmhealey merged commit 4623004 into 0.34 Aug 11, 2026
1 check passed
@hmhealey
hmhealey deleted the MM-69835-4 branch August 11, 2026 15:37
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.

2 participants