Skip to content

feat: Added displayName to React contexts#2512

Merged
bmingles merged 1 commit intodeephaven:mainfrom
bmingles:DH-19529_context-provider-display-name
Aug 5, 2025
Merged

feat: Added displayName to React contexts#2512
bmingles merged 1 commit intodeephaven:mainfrom
bmingles:DH-19529_context-provider-display-name

Conversation

@bmingles
Copy link
Copy Markdown
Contributor

@bmingles bmingles commented Jul 28, 2025

Added displayName to React contexts so that they can be identified in React dev tools.

Part of DH-19529

Testing

  • Run DH web locally via npm start
  • Open Components tab from React dev tools
  • Verify that context providers now have names in the component tree

@bmingles bmingles requested review from a team and mattrunyon and removed request for a team July 28, 2025 16:47
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 28, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.61%. Comparing base (8407e4c) to head (bb46ae8).

Files with missing lines Patch % Lines
packages/chart/src/ChartThemeProvider.tsx 0.00% 1 Missing ⚠️
packages/jsapi-bootstrap/src/ClientBootstrap.tsx 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2512      +/-   ##
==========================================
+ Coverage   44.59%   44.61%   +0.02%     
==========================================
  Files         763      763              
  Lines       42700    42720      +20     
  Branches    10741    10938     +197     
==========================================
+ Hits        19041    19059      +18     
+ Misses      23648    23603      -45     
- Partials       11       58      +47     
Flag Coverage Δ
unit 44.61% <90.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

The branch has a different ticket number from the PR title. I'm also not sure how this change applies to the ticket in branch name or PR title.

We should try to formalize this, but at least what I've been doing if I needed multiple PRs for a fix is not putting the ticket name in the title of the first PR. So web-client-ui doesn't get the ticket number, the plugins repo does so the ticket doesn't get moved to RTT based on just a web-client-ui update.

Also, would we want to enable an eslint rule for this? We already have it for other components. Looks like it's just a boolean option we can enable here

@bmingles bmingles changed the title feat: DH-18999: Added displayName to React contexts feat: DH-19529: Added displayName to React contexts Jul 29, 2025
@bmingles
Copy link
Copy Markdown
Contributor Author

bmingles commented Jul 29, 2025

@mattrunyon I fixed the PR title. I'll leave the Jira in the title for now until we get something more formal. I will just manually move the ticket back for now. Have already been doing so on this particular one.

For the eslint rule, I think that's a good idea, but looks like the version of eslint-plugin-react we are using does not have that option. I attempted it but got an error. Package lock suggests we are on 7.31.11 which doesn't seem to have that option yet:
https://github.com/jsx-eslint/eslint-plugin-react/blob/v7.31.11/docs/rules/display-name.md

I don't want to bring in the scope to upgrade eslint on this PR since I kind of slipped this in to support some sleuthing I was doing.

@bmingles bmingles requested a review from mattrunyon August 4, 2025 14:21
@mattrunyon mattrunyon changed the title feat: DH-19529: Added displayName to React contexts feat: Added displayName to React contexts Aug 5, 2025
Copy link
Copy Markdown
Collaborator

@mattrunyon mattrunyon left a comment

Choose a reason for hiding this comment

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

Asked Bender and he said let's remove the ticket number from the PR title so it doesn't look like the ticket should be moved to RTT when the package update happens in DHE

I put the ticket number in the body though so it has a reference still, but only the title goes in the changelog on release

@bmingles bmingles merged commit 4a9a836 into deephaven:main Aug 5, 2025
16 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 5, 2025
@bmingles bmingles deleted the DH-19529_context-provider-display-name branch August 5, 2025 18:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants