chore(components): upgrade LeafyGreen non-modal (+ some chat related) components COMPASS-9642 #7582

kraenhansen · 2025-11-19T10:09:13Z

Description

This PR upgrades the non-modal and non-chat (although some updates to the chat packages were made) related LG packages to their latests version.

Checklist

New tests and/or benchmarks are included
Documentation is changed or added
If this change updates the UI, screenshots/videos are added and a design review is requested
If this change could impact the load on the MongoDB cluster, please describe the expected and worst case impact
I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

Bugfix
New feature
Dependency update
Misc

Open Questions

Dependents

Types of changes

Backport Needed
Patch (non-breaking change which fixes an issue)
Minor (non-breaking change which adds functionality)
Major (fix or feature that would cause existing functionality to change)

Copilot

Pull Request Overview

This PR upgrades non-modal LeafyGreen UI components across the Compass codebase. The upgrade addresses type incompatibilities, removes deprecated props, updates test selectors to match new component structures, and removes a workaround patch script that is no longer needed.

Key changes:

Upgrades LeafyGreen component versions in package.json files
Adds type parameters and as props to Button and IconButton components for polymorphic type support
Updates test selectors to match new LeafyGreen component test IDs and ARIA attributes
Removes deprecated props like readOnly and adds new type exports

Reviewed Changes

Copilot reviewed 33 out of 34 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
packages/compass-components/package.json	Updates LeafyGreen package versions and removes postinstall script
package.json	Updates root-level LeafyGreen dependencies to match component upgrades
packages/compass-components/src/components/leafygreen.tsx	Updates imports to use named exports and removes TextInput wrapper workaround
packages/compass-components/src/components/links/link.tsx	Adds polymorphic type support to Button and IconButton components
packages/compass-components/src/hooks/use-error-details.tsx	Removes ts-expect-error comment for autoFocus prop
packages/compass-components/scripts/patch-leafygreen-button.js	Removes obsolete patch script for LeafyGreen button package
packages/compass-editor/src/editor.tsx	Adds autoFocus prop support to editor component
packages/compass-e2e-tests/helpers/selectors.ts	Updates test selectors for new LeafyGreen modal close button ID
packages/compass-e2e-tests/helpers/commands/*.ts	Updates E2E test commands to use clickVisible instead of keyboard events
packages/compass-crud/src/components/*.tsx	Removes deprecated readOnly prop and adds data-testid attributes
Other component files	Adds `as="button"` props and updates ref types for polymorphic components

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-19T16:17:30Z

packages/compass-editor/src/editor.tsx

+    if (autoFocusRef.current) {
+      editorViewRef.current?.focus();
+    }
+  }, [autoFocusRef]);


The useLayoutEffect dependency array includes autoFocusRef, but refs don't trigger re-renders. Since autoFocus is passed as a prop and stored in a ref that never changes, this effect should either have an empty dependency array [] or be removed in favor of a simpler approach. The current implementation will only run once anyway, but the dependency is misleading.

Suggested change

}, [autoFocusRef]);

}, []);

@gribnoysup what do you think of this? I vaguely remember talking with you about this at the offsite.

You indeed don't need to provide refs in the dependencies, react-hooks eslint rule also accounts for this (I'm assuming react compiler would do the same). Is it confusing? I don't think it is, but is it somewhat unnecessary. I've no idea what the bot means by simpler approach

Copilot · 2025-11-19T16:17:31Z

packages/compass-components/src/components/links/link.tsx

    ref: React.ForwardedRef<HTMLButtonElement>
  ) => {
    const { utmSource, utmMedium } = useRequiredURLSearchParams();
+    const { href } = rest as { href?: string };


Using type assertion as { href?: string } bypasses TypeScript's type checking. Consider extracting href from rest using destructuring with a type guard or conditional type checking instead of type assertion to maintain type safety.

Copilot · 2025-11-19T16:17:31Z

packages/compass-components/src/components/links/link.tsx

+    ref: React.ForwardedRef<HTMLButtonElement>
  ) => {
    const { utmSource, utmMedium } = useRequiredURLSearchParams();
+    const { href } = rest as { href?: string };


Using type assertion as { href?: string } bypasses TypeScript's type checking. Consider extracting href from rest using destructuring with a type guard or conditional type checking instead of type assertion to maintain type safety.

Copilot · 2025-11-19T16:17:31Z

packages/compass-e2e-tests/helpers/compass.ts

-    });
+    execFile(
+      path,
+      args,


The addition of encoding: 'utf8' changes the function's behavior but lacks explanation. Add a comment explaining why this encoding is now explicitly required for the LeafyGreen upgrade or what issue it addresses.

Suggested change

args,

args,

// Explicitly set encoding to 'utf8' to ensure stdout/stderr are returned as strings.

// This is required for compatibility with the LeafyGreen upgrade, which expects string output.

Copilot · 2025-11-19T16:17:32Z

packages/compass-e2e-tests/tests/assistant.test.ts

+        expect(
+          await browser.$(Selectors.AssistantChatMessages).isDisplayed()
+        ).equals(false);


Use .to.equal(false) instead of .equals(false) for consistency with Chai assertion style used elsewhere in the test file. The current syntax uses the wrong assertion method.

Copilot · 2025-11-19T16:17:32Z

packages/compass-e2e-tests/tests/assistant.test.ts

-        expect(await getDisplayedMessages(browser)).to.deep.equal([]);
+        expect(
+          await browser.$(Selectors.AssistantChatMessages).isDisplayed()
+        ).equals(false);


Use .to.equal(false) instead of .equals(false) for consistency with Chai assertion style used elsewhere in the test file. The current syntax uses the wrong assertion method.

Suggested change

).equals(false);

).to.equal(false);

Copilot · 2025-11-19T16:17:32Z

package.json

+    "@leafygreen-ui/text-area": "^12.1.2",
+    "@leafygreen-ui/card": "^13.2.0",
+    "@leafygreen-ui/logo": "^11.1.0",
+    "@leafygreen-ui/tabs": "^17.0.6",


The addition of the nwsapi dependency lacks context. Add a comment or update the PR description explaining why this specific package is needed for the LeafyGreen upgrade.

Suggested change

"@leafygreen-ui/tabs": "^17.0.6",

"@leafygreen-ui/tabs": "^17.0.6",

// The 'nwsapi' dependency is required for compatibility with upgraded LeafyGreen UI components.

kraenhansen · 2025-11-20T10:24:46Z

packages/compass-e2e-tests/tests/assistant.test.ts

+
+    // Close the drawer if open to provide a clean environment for the next test
+    const drawerCloseButton = browser.$(Selectors.AssistantDrawerCloseButton);
+    if (await drawerCloseButton.isDisplayed()) {
+      await browser.clickVisible(drawerCloseButton);
+      await drawerCloseButton.waitForDisplayed({ reverse: true });
+    }


I'm suggesting closing the drawer (if open) to provide a clean environment between tests.

gribnoysup · 2025-11-20T10:50:06Z

package.json

+    "@leafygreen-ui/card": "^13.2.0",
+    "@leafygreen-ui/logo": "^11.1.0",
+    "@leafygreen-ui/tabs": "^17.0.6",
+    "nwsapi": "2.2.12"


Can you explain what's the context here? I don't see it in any leafygreen transitive deps, in our current package-lock it's on the same version as you locked here as far as I can see. Specifically that we're fixing it to the exact number looks peculiar to me

This is a dependency of jsdom and we were hitting issues with nwsapi@2.2.22 (released only 2 months ago) not being able to parse testid attributes with colons in them.

1) In-Use Encryption supports multiple kms providers from same type allows rename of KMS provider: SyntaxError: 'input#local,,,A1.lg-ui-form-field-input-0000 button.lg-ui-form-field-icon-0000' is not a valid selector

I suspect I was seeing this because I nuked the package lock at some point during this process, which caused the new version of nwsapi to get pulled in (we're on jsdom@24.1.3 which has nwsapi@^2.2.12).

I'll try to remove this override to see if the regression re-appears 🤔 In any case, it's just a package-lock rebuild away from triggering again - so it might be good to get in as a separate PR anyway ...

Got it, no wonder I see so many changes in package-lock that are not related to leafygreen-ui! Let's open a ticket to address this: I guess it can be something like "bump jsdom to latest" so that we don't focus only on transitive dep there?

On a related note, I mentioned this before, but I will do it again just to make sure we're on the same page here:

it's just a package-lock rebuild away from triggering again

This should be a "there's no way around this" kind-of situation (I don't doubt this PR might be one of those rare cases), I think it would be very much worth calling out in the PR description when this happens. Lockfiles are there to make sure that we are not inroducing unexpected version changes in our dependency tree. Changes that big can't even be reviewed via GH UI meaning they always require very special attention.

gribnoysup · 2025-11-20T11:09:09Z

packages/compass-components/package.json

+    "@leafygreen-ui/tokens": "^4.0.0",
+    "@leafygreen-ui/tooltip": "^14.2.2",
+    "@leafygreen-ui/typography": "^22.2.0",
    "@lg-chat/avatar": "^7.0.2",


Are those not "non-modal" components? Looks like latest is 8, not 7. Same question for some other lg-chat stuff, curious why we're not updating those too

You're right - good call-out.

Short answer is I believe these new versions were released since I branched off and I didn't want to complicate things by upgrading these in the process as well. I imagine I'll need to circle back on these once my modal upgrading PR lands 🙄

BTW - some of these also got renamed from the @lg-chat to @leafygreen-ui.

Ah, got it, makes sense, keeping them out sounds okay to me, just wanted to make sure we didn't just accidentally missed them here 👍

I renamed the PR title to reflect this.

gribnoysup

As far as I can tell looking through the package-lock and relying on our e2e-s and running the build locally to confirm that the app is still functioning I think it's LGTM (there's some unexpected unhoisting happening, but that's not the end of the world), but as we changed almost every dependency in our dependency tree (both prod and dev) and there's no way to say exactly what might've changed there I think merging this PR should be followed by doing a Beta release to make sure all release pipelines and resulting app is still working as expected.

Once again want to mention that nuking package-lock is a "break glass" kind of operation. As this patch doing most of the updating it seems, I hope going forward this approach can be avoided.

kraenhansen · 2025-11-24T14:31:51Z

merging this PR should be followed by doing a Beta release to make sure all release pipelines and resulting app is still working as expected.

I'd like to merge and include the other PRs upgrading LG components in that beta release as well - would that work for you?

nuking package-lock is a "break glass" kind of operation.

To be clear, I believe I did this during the process, resulting in some of the changes I'm proposing. I don't however believe that reflects the current state of this PR - the commits upgrading LG components was not the result of deleting and recreating the package-lock entirely.

gribnoysup · 2025-11-24T14:45:48Z

I'm not sure I follow, sorry 🙂 There are a lot of changes in package-lock related to our app build dependencies (various transitive deps of various electron packaging libraries, webpack and some plugins / loaders for example), they wouldn't get bumped with leafygreen update, so there are way more changes in this patch than just related to leafygreen

kraenhansen · 2025-11-24T15:44:24Z

I rebased (as the package-lock got a conflict) and included #7589 and 7590 into this PR (as the changes needed to tests and code was minimal).

This was fixed in LG-5063

mongodb/leafygreen-ui#3218

…hen selecting aggregation stage operator

…lining opt-in

…les with BOM" test

…ing for messages

… compass-indexes

kraenhansen self-assigned this Nov 19, 2025

kraenhansen added dependencies Pull requests that update a dependency file no release notes Fix or feature not for release notes labels Nov 19, 2025

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch 3 times, most recently from 784e3c5 to f130e3d Compare November 19, 2025 14:07

kraenhansen marked this pull request as ready for review November 19, 2025 16:15

kraenhansen requested a review from a team as a code owner November 19, 2025 16:15

kraenhansen requested review from Copilot and ivandevp November 19, 2025 16:15

Copilot AI reviewed Nov 19, 2025

View reviewed changes

kraenhansen requested review from gribnoysup and lerouxb and removed request for ivandevp November 20, 2025 06:53

kraenhansen commented Nov 20, 2025

View reviewed changes

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch from f130e3d to b1946f0 Compare November 20, 2025 10:29

gribnoysup reviewed Nov 20, 2025

View reviewed changes

kraenhansen changed the title ~~chore(components): upgrade LeafyGreen non-modal components COMPASS-9642~~ chore(components): upgrade LeafyGreen non-modal & non-chat components COMPASS-9642 Nov 20, 2025

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch from 0ad7378 to 1cd5f7c Compare November 21, 2025 07:28

This was referenced Nov 21, 2025

chore(components): upgrade LeafyGreen non-modal & non-chat components (again) COMPASS-9642 #7589

Closed

chore(components): upgrade LeafyGreen chat components COMPASS-9642 #7590

Closed

gribnoysup approved these changes Nov 24, 2025

View reviewed changes

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch from 1cd5f7c to c4ed186 Compare November 24, 2025 15:41

kraenhansen changed the title ~~chore(components): upgrade LeafyGreen non-modal & non-chat components COMPASS-9642~~ chore(components): upgrade LeafyGreen non-modal components COMPASS-9642 Nov 24, 2025

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch 2 times, most recently from 66ec1e3 to 8b483ab Compare November 24, 2025 16:10

kraenhansen added 27 commits November 25, 2025 15:15

Use named over default exports

4c35c6a

Fix Button / IconButton links

0bde132

Fix ActionButton prop types

2c213a2

Remove defaultValue workaround

c4b510b

This was fixed in LG-5063

Fix SmallIconButton props

1643455

TEMP: Working around https://jira.mongodb.org/browse/LG-5461

bfcd92c

Workaround for React.ComponentProps<typeof Button> no longer working

f8ab37d

TEMP work around https://jira.mongodb.org/browse/LG-5590

c33b90e

Fix issue passing Popover ref into IconButton

f622a80

Fix check failure in guide-cue spec

bd565a3

Removed extraneous ts-expect-error

66bc1e6

Update to account for breaking change in select v16.2.0

6e99b2c

Wait for crud elements to become visible before interacting

0be5d0a

Update existing tests to handle changes in Select

623bf10

mongodb/leafygreen-ui#3218

Fix index creation interactions with ComboBox

4142956

Update e2e tests to use setComboBoxValue instead of setValueVisible w…

d4d2769

…hen selecting aggregation stage operator

Update e2e tests to expect chat messages to not be displayed when dec…

2090deb

…lining opt-in

Update ConnectDropdownButton to reflect change in SplitButton

d5ae3a0

Update e2e test to use a different selector for menu items in "CSV fi…

b01bf22

…les with BOM" test

Skip opening the assistant drawer if it's already open

f3c65e4

Assistant tests: Add option to useExplainPlanEntryPoint to avoid wait…

c37c211

…ing for messages

Assistant tests: Close drawer after every test.

fac7e14

Export ConfirmationModalProps fixing re-export of showConfirmation in…

d7329d0

… compass-indexes

Update modal close selector

12f7ae9

Update use of chat-provider v6

d276850

Update package lock

b2861ce

Update assistant tests to reflect change in LG

694ac4e

kraenhansen force-pushed the kh/leafygreen-non-modal-updates branch from 7b62353 to 694ac4e Compare November 25, 2025 14:15

kraenhansen merged commit b3d22cc into main Nov 25, 2025
61 checks passed

kraenhansen deleted the kh/leafygreen-non-modal-updates branch November 25, 2025 16:12

-      args,
+      args,
+      // Explicitly set encoding to 'utf8' to ensure stdout/stderr are returned as strings.
+      // This is required for compatibility with the LeafyGreen upgrade, which expects string output.

	"@leafygreen-ui/tabs": "^17.0.6",
	"@leafygreen-ui/tabs": "^17.0.6",
	// The 'nwsapi' dependency is required for compatibility with upgraded LeafyGreen UI components.

chore(components): upgrade LeafyGreen non-modal (+ some chat related) components COMPASS-9642 #7582

chore(components): upgrade LeafyGreen non-modal (+ some chat related) components COMPASS-9642 #7582

Uh oh!

Conversation

kraenhansen commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Motivation and Context

Open Questions

Dependents

Types of changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

gribnoysup Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gribnoysup Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

gribnoysup Nov 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gribnoysup Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gribnoysup Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

kraenhansen Nov 21, 2025

Choose a reason for hiding this comment

Uh oh!

gribnoysup left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kraenhansen commented Nov 19, 2025 •

edited

Loading

kraenhansen Nov 20, 2025 •

edited

Loading

gribnoysup Nov 21, 2025 •

edited

Loading

kraenhansen Nov 20, 2025 •

edited

Loading

kraenhansen Nov 20, 2025 •

edited

Loading

gribnoysup left a comment •

edited

Loading

kraenhansen commented Nov 24, 2025 •

edited

Loading