Skip to content

Conversation

richardkmichael
Copy link
Collaborator

Update the radix-ui/react-* components and the lucide-react icons.

Motivation and Context

Periodic package update reduce the future maintenance effort.

How Has This Been Tested?

After each package update, I checked various UI elements using it against the MCP "everything" server. I also ran the new npm run test:e2e tests.

Breaking Changes

No.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Checked for icon name changes by teaching Claude to search the lucide-react repo with:

`git log -i -E --grep '(rename|change)' 0.448.0^...0.523.0

  ..then searching for any of the found icons in the Inspector imports.
@richardkmichael
Copy link
Collaborator Author

richardkmichael commented Jun 24, 2025

I noticed dependabot has updated vite occasionally, but it's not doing other dependencies? I'd be happy to try and update jest, which should eliminate the deprecation warnings; and tailwind before it drifts too far.

The Playwright e2e tests can take longer than 5 minutes (GitHub Actions CI load maybe?), so I increased the timeout. I was experimenting here, but I can make a separate PR.

- Playwright installation can take more than 5m, increase to 15m

- Only attempt to upload the report if Playwright executed, otherwise
  there are no report files, and the upload step will error
@richardkmichael richardkmichael force-pushed the update-radix-and-icons branch from 34c619e to 36e053a Compare June 25, 2025 09:05
test:
timeout-minutes: 5
# Installing Playright dependencies can take quite awhile, and also depends on GitHub CI load.
timeout-minutes: 15
Copy link
Member

Choose a reason for hiding this comment

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

Wow, I am kind of surprised that CI might take this long for these tests. Wonder if this should only run on main or on demand?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it can run on all PRs, as-is.

The actual tests take only ~25 seconds. A total test run is ~3m20s. Both visible on this successful run.

The failed run exceeded the timeout during runner prep; apt-get update and Playwright install w deps took > 4 minutes. I'm not sure, but presume this depends on cache state and GitHub infrastructure load, so the minimum is difficult to predict. 10 minutes might be enough, I set it to 15 to avoid fiddling with it. :-)

Ideally the author runs locally too. Personally, I prefer to have full test suites enabled for PRs / branches, to avoid discovering breakage after a merge. If the test suite is slow, that ought to be dealt with.

@olaservo
Copy link
Member

Thanks for the PR and comments on the dependency updates. I was thinking about something similar for MCP SDK updates in the reference servers repo since a lot tends to drift out of date. So I think we could create a followup issue for improving on that for other packages here as well.

@olaservo olaservo merged commit d29d707 into modelcontextprotocol:main Jun 25, 2025
4 checks passed
@richardkmichael
Copy link
Collaborator Author

Thanks for the PR and comments on the dependency updates. I was thinking about something similar for MCP SDK updates in the reference servers repo since a lot tends to drift out of date. So I think we could create a followup issue for improving on that for other packages here as well.

I'm unsure why dependabot isn't doing more. I might investigate that at some point, I'm not very familiar with it.

@richardkmichael richardkmichael deleted the update-radix-and-icons branch June 25, 2025 19:48
@cliffhall
Copy link
Member

@richardkmichael Catching this after the fact, but why does package-lock.json contain lots of changes to radix-ui components but the package.json only contains changes to lucide-react?

@richardkmichael
Copy link
Collaborator Author

@cliffhall 👋

The changes to the radix-ui components in the lockfile are just from running npm update to respect the existing version ranges in package.json.

Regarding lucide-react, because it’s a 0.x version, npm update will only ever apply patches. However, they don't seem to release patches (fixes are in new minor versions) and it moves quickly, meaning it was far behind. So I figured it would be worth tackling.

I manually bumped package.json and checked that no used icons were affected. In case it would be helpful for future updates, I explained the process in the commit message.

Do you prefer to "pull-up" package.json versions to those of the lockfile?

@cliffhall
Copy link
Member

The changes to the radix-ui components in the lockfile are just from running npm update to respect the existing version ranges in package.json.

Thanks, @richardkmichael.

The weird thing about it is that none of the radix-ui components have been touched for a minimum of 2 months.
Screenshot 2025-06-30 at 3 28 26 PM

Further, at least some the versions that are in client/package.json don't match either the new versions in package-lock.json OR the versions they were updated from.

So... I think syncing them is in order.

The reason I'm coming back to this is that a report of some weird behavior was posted in our private Discord

CleanShot_2025-06-30_at_15.23.51.mp4

While I can't duplicate it myself, the only changes in 0.15.0 that I can see that could have possibly led to them was this PR.
Screenshot 2025-06-30 at 3 30 08 PM

@richardkmichael
Copy link
Collaborator Author

@cliffhall

The changes to the radix-ui components in the lockfile are just from running npm update to respect the existing version ranges in package.json.

Further, at least some the versions that are in client/package.json don't match either the new versions in package-lock.json OR the versions they were updated from.

I'm happy to investigate, but I don't quite understand. Can you please show me what you mean? An example of an unexpected mismatch would be helpful.

So... I think syncing them is in order.

Yes, I also prefer the minimum package declarations in a project file to match the lockfile. I have other updates coming; after or as part of that I could reconcile package.json with package-lock.json. It shouldn't change anything of course, it'll just be clearer when reading package.json.

The reason I'm coming back to this is that a report of some weird behavior was posted in our private Discord
CleanShot_2025-06-30_at_15.23.51.mp4

While I can't duplicate it myself, the only changes in 0.15.0 that I can see that could have possibly led to them was this PR

The sidebar bizarrely resizing from the center is a difference between Chrome (janky) and Firefox. I tracked it down to between 0.14.0 and 0.14.1. git log 0.14.0..0.14.1 has me guess it was the sidebar resize PR #459

Screen recording, Firefox on the left, Chrome on the right.
https://drive.google.com/file/d/1BtdU_NpdFtHm298XjwIUfUw5ioTiCYUt/view?usp=sharing

@cliffhall
Copy link
Member

The sidebar bizarrely resizing from the center is a difference between Chrome (janky) and Firefox. I tracked it down to between 0.14.0 and 0.14.1. git log 0.14.0..0.14.1 has me guess it was the sidebar resize PR #459

Screen recording, Firefox on the left, Chrome on the right.

Screenshot 2025-07-01 at 11 40 08 AM

But in your video, even the 0.14.0 screen was scrunched in the middle, when the sidebar was not adjustable. Seems like that part was broken even before?

@richardkmichael richardkmichael mentioned this pull request Jul 1, 2025
9 tasks
@richardkmichael
Copy link
Collaborator Author

@cliffhall I thought the broken part was the resizing behaviour. I only use Firefox, so I'm unsure if/when the Chrome alignment was ever on the left and became (broke?) centered. The fix is to use w-full on #root. Also, margin: 0 auto; has no effect and can be removed. PR #581

@richardkmichael richardkmichael mentioned this pull request Jul 26, 2025
9 tasks
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.

3 participants