refactor(ui): use new useClipboard with async#2675
refactor(ui): use new useClipboard with async#2675MatteoGabriele wants to merge 1 commit intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughThe PR replaces a custom ChangesClipboard API Migration
Possibly Related PRs
Suggested Reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
📊 Dependency Size ChangesWarning This PR adds 1.3 MB of new dependencies, which exceeds the threshold of 200 kB.
Total size change: 1.3 MB |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/pages/package/`[[org]]/[name].vue:
- Around line 119-124: The copyReadme helper currently calls copy with an empty
string when fetchReadmeMarkdown fails; change copyReadme so it awaits
fetchReadmeMarkdown, checks readmeMarkdownData.value?.markdown and if it's falsy
returns early (or surfaces an error/toast) instead of calling copy with ''—use
fetchReadmeMarkdown(), inspect readmeMarkdownData (or its Markdown field) and
only call copy(async () => ...) when markdown is non-empty, otherwise bail out
or raise a user-visible error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 81213369-668c-43bc-b6d3-2faddd6afa3e
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
app/composables/useClipboardAsync.tsapp/pages/package/[[org]]/[name].vuepackage.json
💤 Files with no reviewable changes (1)
- app/composables/useClipboardAsync.ts
| function copyReadme() { | ||
| copy(async () => { | ||
| await fetchReadmeMarkdown() | ||
| return readmeMarkdownData.value?.markdown ?? '' | ||
| }, | ||
| { | ||
| copiedDuring: 2000, | ||
| }, | ||
| ) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Don't copy an empty README on fetch failure.
The helper currently falls back to '' when the markdown ref is empty, so a failed fetch can still look like a successful copy. Return early or surface an error instead of treating missing markdown as valid content.
🛠 Suggested fix
async function copyReadme() {
copy(async () => {
await fetchReadmeMarkdown()
- return readmeMarkdownData.value?.markdown ?? ''
+ const markdown = readmeMarkdownData.value?.markdown
+ if (!markdown) throw new Error('README Markdown is unavailable')
+ return markdown
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/pages/package/`[[org]]/[name].vue around lines 119 - 124, The copyReadme
helper currently calls copy with an empty string when fetchReadmeMarkdown fails;
change copyReadme so it awaits fetchReadmeMarkdown, checks
readmeMarkdownData.value?.markdown and if it's falsy returns early (or surfaces
an error/toast) instead of calling copy with ''—use fetchReadmeMarkdown(),
inspect readmeMarkdownData (or its Markdown field) and only call copy(async ()
=> ...) when markdown is non-empty, otherwise bail out or raise a user-visible
error.
ghostdevv
left a comment
There was a problem hiding this comment.
assuming the code rabbit comment is valid, we can fix it here or create an issue? otherwise LGTM
@ghostdevv the copying action "never fails", even if it does. |
shuuji3
left a comment
There was a problem hiding this comment.
Works as expected on Firefox/Google Chrome on Linux/macOS/Android ✨ 🧹
The fix for #2151 has been incorporated into vueuse, and with the release of v14.3.0, we can now remove the additional code and rely on the version included in the library.
see vueuse/vueuse#5368