-
-
Notifications
You must be signed in to change notification settings - Fork 241
fix: remove html comments from package description and deprecation notices #1397
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
fix: remove html comments from package description and deprecation notices #1397
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
📝 WalkthroughWalkthroughThe changes modify the 🚥 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)
No actionable comments were generated in the recent review. 🎉 Comment |
app/composables/useMarkdown.ts
Outdated
| stripped = stripped.replace(/<\/?[a-z][^>]*>/gi, '') | ||
|
|
||
| // Strip HTML comments: <!-- ... --> | ||
| stripped = stripped.replace(/<!--[\s\S]*?-->/g, '') |
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.
As far as I understand, when there's no description in package.json, npm and algolia takes the description from Markdown. If the comment is long, only the opening tag will remain. What do you think about additional check so that in this case it's truncated to the end?
p.p.s. just note - [\s\S]* is equal to [\s\S]{0,}, i.e. * means that there are more than 0 matches of any character. Therefore, the optional character (?) is not needed here. The optional character may be needed when testing with + instead of *, that equal to [\s\S]{1,} (at least one any character)
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.
Ah, I was consider adding it but didn't know how the description is retrieved. Let me update the logic.
Note on the regex: *? is used to be non-greedy and not match more than wanted.
Resolves #1354
Not ideal as packages might have no description then but given that the description is fetched directly from NPM, we could only parse the README and take the first non-comment line.
This could be done in another iteration if worth it