feat: add deprecate command with custom reason #921
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
📝 WalkthroughWalkthroughThis pull request introduces package deprecation functionality with custom reasons. It adds a new Vue modal component for the deprecation workflow, integrates it into the package page to allow package owners to deprecate packages or versions, implements a new npm-client function to execute deprecation via the npm CLI, extends operation types and schemas to support the 'package:deprecate' operation, updates the server to handle this operation type, and provides localisation strings in multiple languages including English and Simplified Chinese. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
app/pages/package/[...package].vue (1)
1156-1169: Remove duplicate CSS classes.The button has redundant
inline-flex items-center(conflicting withflex) andw-fullappearing twice in the class string.🧹 Proposed fix
<button type="button" - class="flex items-center justify-center w-full px-3 py-1.5 bg-bg-subtle rounded text-sm font-mono text-red-400 hover:text-red-500 transition-colors inline-flex items-center gap-1.5 w-full" + class="flex items-center justify-center gap-1.5 w-full px-3 py-1.5 bg-bg-subtle rounded text-sm font-mono text-red-400 hover:text-red-500 transition-colors" `@click`="deprecateModal?.open()" >
| const command = params.version | ||
| ? `npm deprecate ${props.packageName}@${params.version} "${message}"` | ||
| : `npm deprecate ${props.packageName} "${message}"` |
There was a problem hiding this comment.
Escape quotes in the deprecation message to prevent command parsing issues.
If a user enters a message containing double quotes (e.g., Use "package-x" instead), the generated command will have unbalanced quotes, potentially causing the CLI operation to fail or behave unexpectedly.
🛡️ Proposed fix
+ // Escape double quotes in message for shell safety
+ const escapedMessage = message.replace(/"/g, '\\"')
+
const command = params.version
- ? `npm deprecate ${props.packageName}@${params.version} "${message}"`
- : `npm deprecate ${props.packageName} "${message}"`
+ ? `npm deprecate ${props.packageName}@${params.version} "${escapedMessage}"`
+ : `npm deprecate ${props.packageName} "${escapedMessage}"`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const command = params.version | |
| ? `npm deprecate ${props.packageName}@${params.version} "${message}"` | |
| : `npm deprecate ${props.packageName} "${message}"` | |
| // Escape double quotes in message for shell safety | |
| const escapedMessage = message.replace(/"/g, '\\"') | |
| const command = params.version | |
| ? `npm deprecate ${props.packageName}@${params.version} "${escapedMessage}"` | |
| : `npm deprecate ${props.packageName} "${escapedMessage}"` |
| deprecateError.value = completedOp.result?.stderr || t('deprecate.modal.failed') | ||
| } |
There was a problem hiding this comment.
Missing i18n key for error fallback.
The key deprecate.modal.failed does not exist in any locale file. This will display the raw key string to users when an error occurs.
You need to either:
- Add a
failedkey underpackage.deprecation.modalin all locale files, or - Use an existing key like
t('common.try_again')or a hardcoded fallback.
🐛 Proposed fix (option 1 - use correct path and add key)
} else {
- deprecateError.value = completedOp.result?.stderr || t('deprecate.modal.failed')
+ deprecateError.value = completedOp.result?.stderr || t('package.deprecation.modal.failed')
}Then add to each locale file under package.deprecation.modal:
"failed": "Failed to deprecate package"| } catch (err) { | ||
| deprecateError.value = err instanceof Error ? err.message : t('deprecate.modal.failed') | ||
| } finally { |
There was a problem hiding this comment.
Same missing i18n key in catch block.
This catch block also references the non-existent deprecate.modal.failed key.
🐛 Proposed fix
} catch (err) {
- deprecateError.value = err instanceof Error ? err.message : t('deprecate.modal.failed')
+ deprecateError.value = err instanceof Error ? err.message : t('package.deprecation.modal.failed')
} finally {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (err) { | |
| deprecateError.value = err instanceof Error ? err.message : t('deprecate.modal.failed') | |
| } finally { | |
| } catch (err) { | |
| deprecateError.value = err instanceof Error ? err.message : t('package.deprecation.modal.failed') | |
| } finally { |
1 participant
resolve #40