-
Notifications
You must be signed in to change notification settings - Fork 10
fix(deps): update lucide monorepo (major) #911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ | |
| "clsx": "^2.1.1", | ||
| "cmdk": "^1.1.1", | ||
| "date-fns": "^4.1.0", | ||
| "lucide-react": "^0.577.0", | ||
| "lucide-react": "^1.0.0", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-react update to v1 is safe. According to the official migration guide, the only breaking change in v1 is the removal of brand icons (GitHub, Facebook, Twitter, etc.), and none of those icons are used in this codebase. The icon API and props remain unchanged. Why it matters: Major version bumps can introduce breaking changes. In this case, the migration is straightforward because:
Suggested fix: No change needed to this line. However, ensure the full test suite passes after the update. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-react upgrade from 0.577.0 to ^1.0.0 (resolves to 1.14.0) is safe for this codebase. Why it matters: Lucide v1 removed several brand icons (Github, Gitlab, Facebook, etc.) as documented in the migration guide. I verified via grep that none of these removed icons are imported anywhere in the codebase. Additionally, v1 now sets Suggested fix: No action required, but consider pinning to a specific version (e.g., There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-icons maintainers noted that v1.0.0 was "published unintentionally" and recommended using v1.0.1+ instead (see release notes). Why it matters: Using an unintentionally published version may indicate potential instability or issues that were fixed in subsequent patch releases. Suggested fix: Consider updating to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Major version upgrade from
The lockfile resolves to Suggested verification: Run the frontend dev server and verify all icons render correctly, especially in modals, dropdowns, and tables where icons are heavily used. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Major version upgrade from v0.577.0 to v1.x. Why it matters: Lucide v1 removed several brand icons (Github, Figma, Slack, LinkedIn, etc.) and now sets Suggested fix: No action needed, but verify all icons render correctly after merge. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider pinning to a more specific version or verifying compatibility with lucide-react v1 breaking changes. Why it matters: The lucide-react v1 release includes breaking changes:
Since this uses Suggested fix: No action required if testing confirms all icons render correctly. Consider adding a note in changelog about the aria-hidden behavior change if accessibility testing reveals any issues. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider using a more specific version like Why it matters: While functionally equivalent today, being explicit about the actual minimum version you've tested against is a good practice for dependency management. Suggested fix: Change to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider using a more precise version specifier like Why it matters: Using Suggested fix: Update to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Major version upgrade from Why it matters: Lucide v1 includes breaking changes such as removed brand icons and renamed icons ( The v1 release also includes improvements like Suggested fix: No action required, but consider pinning to a specific minor version (e.g., There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-react v1 upgrade looks safe. Why it matters: I verified that none of the 14 removed brand icons (Chromium, Codepen, Codesandbox, Dribbble, Facebook, Figma, Framer, Github, Gitlab, Instagram, LinkedIn, Pocket, RailSymbol, Slack) are used anywhere in the codebase. The main breaking change affecting this project is that Suggested fix: No code changes needed. Consider testing the dashboard visually to confirm all icons render correctly, especially if any custom styling was applied to icon containers. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: The lucide-react update from v0.577.0 to v1.x is clean — verified that none of the removed brand icons (Facebook, GitHub, Slack, etc.) are used in the codebase. Note that per the Lucide v1 migration guide, the following brand icons were removed: Chromium, Codepen, Codesandbox, Dribbble, Facebook, Figma, Framer, Github, Gitlab, Instagram, LinkedIn, Pocket, RailSymbol, Slack. Grep confirmed none of these are imported. The text references to "Slack" and "github" in tests are just string data (webhook URLs, usernames), not icon imports. One behavioral change to be aware of: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Major version bump from 0.577.0 to 1.0.0. Why it matters: Lucide v1.0.0 removed all brand icons (Github, Facebook, Twitter, etc.) as a breaking change. Fortunately, searching the codebase shows no brand icons are currently imported, so this won't break existing functionality. Suggested fix: Consider pinning to a more specific version like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-react v1 upgrade is safe for this codebase. Why it matters: Lucide v1 removed all brand icons (GitHub, Facebook, Instagram, LinkedIn, Dribbble, Figma, Framer, Slack, Pocket, Codepen, Codesandbox, Chromium, RailSymbol) due to trademark concerns. However, I've verified the codebase only uses generic icons like The other v1 changes ( Suggested fix: No action needed — this upgrade is compatible. Just ensure the pnpm overrides issue (separate comment) is resolved first. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: Version specifier Why it matters: According to the official Lucide release notes, v1.0.0 "was published unintentionally" and the maintainers explicitly state "We've corrected this in v1.0.1, which should be used instead." While the lockfile correctly pins 1.17.0, anyone doing a fresh install without the lockfile (or with Suggested fix: Change the version specifier to "lucide-react": "^1.17.0",There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-react upgrade from v0.577.0 to v1.x appears safe based on the release notes. All currently used icons in the codebase (CheckIcon, X, ChevronDown, etc.) remain available in v1.17.0 with the same API. The peer dependency range includes React 19 which you're using. Why it matters: Major version upgrades can sometimes introduce breaking changes. However, lucide-react v1.x maintains backward compatibility with the icon component API and exports. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider updating to Why it matters: According to the official release notes, version 1.0.0 "was published unintentionally" and the maintainers recommend using v1.0.1 instead. While 1.0.1 is primarily a corrective release, using the intended stable version is better practice. Suggested fix: Change the version specifier to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The lucide-react upgrade to v1 appears safe. Per the migration guide (https://lucide.dev/guide/react/migration), v1 removes brand icons, but I verified the codebase doesn't import any of the removed icons (GitHub, Facebook, Instagram, LinkedIn, Twitter, Dribbble, Figma, Framer, Codepen, Codesandbox, Chromium, Pocket, RailSymbol, Slack). Why it matters: The PR title says "update dependency lucide-react to v1" but the lockfile changes include much more than just this dependency update. The scope creep in the lockfile is the real concern here. Suggested fix: No change needed to this line itself, but ensure the lockfile regeneration preserves the pnpm overrides. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking: This version tag should be Why it matters: According to the official Lucide release notes, v1.0.0 was "published unintentionally" and the maintainers explicitly state that v1.0.1 "should be used instead". While v1.0.0 may function correctly, pinning to an unintentionally-published version could miss critical fixes that were included in v1.0.1. Source: Lucide v1.0.0 Release Notes: "Warning: This release was published unintentionally. We've corrected this in v1.0.1, which should be used instead." Suggested fix: Change the version constraint to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Major version upgrade from v0.577.0 to v1.x is acceptable. Why it matters: According to the Lucide v1 migration guide, the only breaking change in v1.0 is the removal of brand icons (Github, Facebook, Figma, Framer, Instagram, LinkedIn, Pocket, Slack, Dribbble, Codepen, Codesandbox, Chromium, Gitlab). A grep search confirms none of these icons are imported in this codebase. Suggested fix: No fix needed, but verify all icon imports still work by running the frontend dev server and checking for any console warnings about missing icons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider updating to Why it matters: According to the v1.0.0 release notes, version 1.0.0 was "published unintentionally" and the maintainers recommend using v1.0.1 instead. While v1.0.0 functions correctly, using the intended first stable release is better practice. Suggested fix: Change to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider using a more specific version like Why it matters: While Suggested fix: Update to: "lucide-react": "^1.17.0",Or keep as-is if you prefer the looser constraint - functionally equivalent since pnpm locked it to 1.17.0. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider using Why it matters: According to the official release notes, v1.0.0 was "published unintentionally" and v1.0.1 was released as the corrected version. While npm's semver resolution with Suggested fix: Change to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: The version specifier Why it matters: The maintainers explicitly state "This release was published unintentionally. We've corrected this in v1.0.1, which should be used instead." While the lockfile resolves to 1.18.0 (which is fine), the specifier should ideally point to a valid minimum version. Suggested fix: Update to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider using Why it matters: According to the v1.0.0 release notes, version 1.0.0 was "published unintentionally" and the maintainers corrected this in v1.0.1. While v1.0.0 works, using the intentional release is better practice. Suggested fix: Change specifier to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Major version upgrade from 0.577.0 to 1.x. Why it matters: Lucide v1 removed all brand icons (Github, Facebook, Instagram, LinkedIn, etc.) as documented in their migration guide. However, I've verified the codebase doesn't use any of these removed icons. Suggested fix: No action needed for icon compatibility. Just ensure visual regression testing passes since some icons may have subtle design changes between versions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: The lucide-react 1.0.0 release was published unintentionally according to the maintainers, who recommend using v1.0.1+ instead. Why it matters: Since the semver range is Suggested fix: No action required — the resolution to 1.21.0 is correct. Consider updating the comment in your headnote that 1.0.0 was accidentally published, but the caret range handles this correctly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking: Consider using Why it matters: According to the v1.0.0 release notes, version 1.0.0 was "published unintentionally" and v1.0.1 should be used instead. While Suggested fix: Change to |
||
| "next-themes": "^0.4.6", | ||
| "openai": "^6.0.0", | ||
| "react": "^19.1.1", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Consider being more explicit about the minimum version.
Why it matters: Using
^1.0.0allows pnpm to auto-update to any 1.x version. While the lockfile pins 1.14.0 for reproducible builds, being explicit about the minimum tested version helps future maintainers understand what version was actually tested.Since 1.0.0 was an accidental release, specifying
^1.0.0might give the impression that 1.0.0 was intentionally targeted.Suggested fix: Consider updating to
"^1.14.0"or at minimum"^1.0.1"(to skip the accidental 1.0.0 release) to be explicit about the minimum version with known compatibility.