Skip to content

feat: add deprecate command with custom reason - #921

Open
eryue0220 wants to merge 38 commits into
npmx-dev:mainfrom
eryue0220:feat/resolve-40
Open

feat: add deprecate command with custom reason#921
eryue0220 wants to merge 38 commits into
npmx-dev:mainfrom
eryue0220:feat/resolve-40

Conversation

@eryue0220

Copy link
Copy Markdown
Contributor

resolve#40

@vercel

vercelBot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentActionsUpdated (UTC)
docs.npmx.devReadyReadyPreviewAug 6, 2026 7:48am
npmx.devReadyReadyPreviewAug 6, 2026 7:48am
1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
npmx-lunariaIgnoredIgnoredAug 6, 2026 7:48am

Request Review

@github-actions

github-actionsBot commented Feb 4, 2026

Copy link
Copy Markdown

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

FileNote
i18n/locales/en.jsonSource changed, localizations will be marked as outdated.
i18n/locales/zh-CN.jsonLocalization changed, will be marked as complete. 🔄️
Warnings reference
IconDescription
🔄️The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@coderabbitai

coderabbitaiBot commented Feb 4, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.
📝 Walkthrough

Walkthrough

This PR introduces package deprecation functionality allowing users to deprecate npm packages or specific versions with custom messages. It includes a new Vue 3 modal component for the deprecation workflow, CLI support via a new packageDeprecate() function, server-side operation handling for the package:deprecate operation type, schema validation for deprecation parameters, internationalization strings in English and Chinese, comprehensive component and a11y tests, and integration into the package detail page with ownership checks and version-aware UI.

Possibly related PRs

  • refactor: separate npm composables #827: Extracted and moved npm API utilities (fetchAllPackageVersions and related functions) to app/utils/npm/api that the deprecation modal component relies on for fetching version data.

Suggested reviewers

  • danielroe
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check nameStatusExplanation
Description check✅ PassedThe pull request description 'resolve #40' directly references the linked issue and indicates the changeset addresses package deprecation functionality.
Linked Issues check✅ PassedThe pull request implements package deprecation with custom reason via the connector across frontend and CLI components, fulfilling the requirements in issue #40.
Out of Scope Changes check✅ PassedAll changes are scoped to package deprecation functionality: UI component, CLI operations, schemas, types, internationalisation, and tests. One minor addition to .gitignore for test coverage files appears incidental but acceptable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecovBot commented Feb 4, 2026

Copy link
Copy Markdown

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 with flex) and w-full appearing 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()"
>

Comment threadapp/components/Package/DeprecatePackageModal.vue Outdated
Comment threadapp/components/Package/DeprecatePackageModal.vue Outdated
Comment threadapp/components/Package/DeprecatePackageModal.vue

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/pages/package/[...package].vue (1)

1245-1256: Clean up duplicate CSS classes.

The button has redundant classes: both flex and inline-flex, and w-full appears twice.

♻️ Suggested 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()"
>

Comment threadcli/src/npm-client.ts

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/components/Package/DeprecatePackageModal.vue (2)

24-28: Use the imported t function instead of $t in the script block.

The component destructures t from useI18n() on line 12, but this computed uses $t. For consistency within the script setup, use the imported t function.

♻️ Proposed fix
 const modalTitle = computed(() =>
deprecateVersion.value
- ? `${$t('package.deprecation.modal.title')} ${props.packageName}@${deprecateVersion.value}`- : `${$t('package.deprecation.modal.title')} ${props.packageName}`,+ ? `${t('package.deprecation.modal.title')} ${props.packageName}@${deprecateVersion.value}`+ : `${t('package.deprecation.modal.title')} ${props.packageName}`,
)

121-127: Consider removing inline focus-visible utilities from buttons.

Per project convention, focus-visible styling for buttons is applied globally via main.css with button:focus-visible { outline: 2px solid var(--accent); ... }. The inline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50 utilities override this global rule.

If this modal requires different focus styling, consider whether it should be consistent with the rest of the application or if an exception is warranted.

Based on learnings: "In the npmx.dev project, ensure that focus-visible styling for button and select elements is implemented globally in app/assets/main.css... Do not apply per-element inline utility classes."

Comment threadapp/components/Package/DeprecatePackageModal.vue Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Comment threadcli/src/npm-client.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
app/components/Package/DeprecatePackageModal.vue (2)

69-69: Consider adding optional chaining for defensive access.

If state.value.operations is ever undefined (e.g., during initialisation or after an error), this line would throw. Adding optional chaining aligns with the strict type-safety guideline.

🛡️ Proposed fix
- const completedOp = state.value.operations.find(op => op.id === operation.id)+ const completedOp = state.value.operations?.find(op => op.id === operation.id)

122-128: Remove inline focus-visible utilities on buttons to rely on global styling.

Both buttons in this component have custom focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50 classes. The project uses a global CSS rule for button focus-visible styling in main.css. Removing these inline utilities ensures consistency across the codebase.

This applies to the button on lines 164-175 as well.

♻️ Proposed fix
 <button
type="button"
- class="w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50"+ class="w-full px-4 py-2 font-mono text-sm text-fg-muted bg-bg-subtle border border-border rounded-md transition-colors duration-200 hover:text-fg hover:border-border-hover"
`@click`="close"
>

And for the deprecate button:

 <button
type="button"
:disabled="isDeprecating || !deprecateMessage.trim()"
- class="w-full px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md transition-colors duration-200 hover:bg-fg/90 disabled:opacity-50 disabled:cursor-not-allowed focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-fg/50"+ class="w-full px-4 py-2 font-mono text-sm text-bg bg-fg rounded-md transition-colors duration-200 hover:bg-fg/90 disabled:opacity-50 disabled:cursor-not-allowed"
`@click`="handleDeprecate"
>

Based on learnings: "In the npmx.dev project, focus-visible styling for buttons and selects is applied globally via main.css... individual buttons or selects in Vue components should not rely on inline focus-visible utility classes."

Comment threadapp/components/Package/DeprecatePackageModal.vue
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…feat/resolve-40
* 'feat/resolve-40' of github.com:eryue0220/npmx.dev:
Update app/components/Package/DeprecatePackageModal.vue
@MatteoGabriele

Copy link
Copy Markdown
Member

@eryue0220, unfortunately I cannot manage to make this work locally

Screenshot 2026-05-09 at 22 28 03

@gameroman@serhalp Did you guys manage? Perhaps I'm doing something wrong in my setup.

@gameroman
gameroman self-requested a review May 11, 2026 07:50
@gameroman
gameroman dismissed their stale reviewMay 11, 2026 07:51

need to re-review

@eryue0220

Copy link
Copy Markdown
ContributorAuthor

@eryue0220, unfortunately I cannot manage to make this work locally

Screenshot 2026-05-09 at 22 28 03@gameroman@serhalp Did you guys manage? Perhaps I'm doing something wrong in my setup.

Could you add more information about this, such as the error details in the terminal or browser logs?

@MatteoGabriele

MatteoGabriele commented May 13, 2026

Copy link
Copy Markdown
Member

@eryue0220 good old 400 in the console and nothing in the terminal
Screenshot 2026-05-13 at 19 00 50

@gameroman

Copy link
Copy Markdown
Member

@gameroman@serhalp Did you guys manage? Perhaps I'm doing something wrong in my setup.

I didn't actually try it locally yet 🙈

@gameroman

gameroman commented May 13, 2026

Copy link
Copy Markdown
Member

@eryue0220 good old 400 in the console and nothing in the terminal Screenshot 2026-05-13 at 19 00 50

Looks like it happens here

const newOperation:NewOperation= {
type: 'package:deprecate',
params: operationParams,
description: parsed.output.version
?`Deprecate ${parsed.output.pkg}@${parsed.output.version}`
:`Deprecate ${parsed.output.pkg}`,
command,
}
const operation =awaitaddOperation(newOperation)

asyncfunctionconnectorFetch<T>(
path: string,
options: {method?: 'GET'|'POST'|'DELETE';body?: Record<string,unknown>}={},
): Promise<T|null>{
if(!config.value)returnnull
try{
constresponse=await$fetch(`${baseUrl.value}${path}`,{
method: options.method??'GET',
headers: {
Authorization: `Bearer ${config.value.token}`,
},
body: options.body,
timeout: 30000,
})
returnresponseasT
}catch(err){
state.value.error=errinstanceofError ? err.message : 'Request failed'
returnnull
}
}
// Operation management
asyncfunctionaddOperation(operation: NewOperation): Promise<PendingOperation|null>{
constresponse=awaitconnectorFetch<ApiResponse<PendingOperation>>('/operations',{
method: 'POST',
body: operationasunknownasRecord<string,unknown>,
})
if(response?.success&&response.data){

@eryue0220

Copy link
Copy Markdown
ContributorAuthor

@eryue0220 good old 400 in the console and nothing in the terminal Screenshot 2026-05-13 at 19 00 50

@MatteoGabriele@gameroman Thanks for your feedback, I checked the error and I need to know you also run the pnpm npmx-connector in your terminal, if not then it will throw this error.

@MatteoGabriele

Copy link
Copy Markdown
Member

@eryue0220 good old 400 in the console and nothing in the terminal Screenshot 2026-05-13 at 19 00 50

@MatteoGabriele@gameroman Thanks for your feedback, I checked the error and I need to know you also run the pnpm npmx-connector in your terminal, if not then it will throw this error.

I do

@gameroman

Copy link
Copy Markdown
Member

@eryue0220 good old 400 in the console and nothing in the terminal Screenshot 2026-05-13 at 19 00 50

@MatteoGabriele@gameroman Thanks for your feedback, I checked the error and I need to know you also run the pnpm npmx-connector in your terminal, if not then it will throw this error.

I do

From local clone? Or from npm?

@MatteoGabriele

Copy link
Copy Markdown
Member

I'm not entirely sure what you mean, but I follow these steps:

  • I run "pnpm npmx-connector"
  • then open the browser
  • authenticate
  • return to npmx
  • paste the token
  • connect
  • see that the deprecate button is available
  • click it to deprecate
  • and then nothing happens and get that error

@gameroman

Copy link
Copy Markdown
Member

I'm not entirely sure what you mean, but I follow these steps:

  • I run "pnpm npmx-connector"
  • then open the browser
  • authenticate
  • return to npmx
  • paste the token
  • connect
  • see that the deprecate button is available
  • click it to deprecate
  • and then nothing happens and get that error

I mean when you run pnpm npmx-connector, do you run it from local clone of this branch, or just the version of npmx-connector that is published on npm?

@gameroman

Copy link
Copy Markdown
Member

@eryue0220 should I reopen this?

@eryue0220

Copy link
Copy Markdown
ContributorAuthor

@eryue0220 should I reopen this?

Sure, it's okay.

@gameromangameroman reopened this Aug 5, 2026
@gameromangameroman removed stale This has become stale and may be closed soon stale-to-close labels Aug 5, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs reviewThis PR is waiting for a review from a maintainerstaleThis has become stale and may be closed soon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

add package deprecation with custom reason

7 participants

@eryue0220@MatteoGabriele@43081j@gameroman@ghostdevv@serhalp@danielroe