Skip to content

feat: add docs, allow URLs & more - #255

Open
WarningImHack3r wants to merge 3 commits into
jpmonette:mainfrom
WarningImHack3r:fix-id-url-and-docs
Open

feat: add docs, allow URLs & more#255
WarningImHack3r wants to merge 3 commits into
jpmonette:mainfrom
WarningImHack3r:fix-id-url-and-docs

Conversation

@WarningImHack3r

Copy link
Copy Markdown

Thanks for this library! I primarily wanted to fix the "id being parsed as a URL" issue and add docs to avoid having to open GitHub to understand the library, but my docs adjustments led to further (non-breaking) improvements.
I can understand if it's too much for one PR to review; don't hesitate if you want me to split it up. I'm reactive to feedback!

Changes

  • Add README-originated doc to user-facing properties
  • Allow using URL in addition to string where relevant
  • Sanitize URLs in more places, improving behavior consistency
  • Fix item ID being parsed as a URL since v6
  • Fix empty author arrays throwing

- add README-originated doc to user-facing properties
- allow using `URL` in addition to `string` where relevant
- sanitize URLs at more places, improving behavior consistency
- fix item id being parsed as an URL since v6
- fix empty author arrays throwing
Comment thread src/atom1.ts
const entry: convert.ElementCompact = {
title: { _attributes: { type: "html" }, _cdata: item.title },
id: sanitizeUrl(item.id ?? item.link),
id: sanitize(item.id) ?? sanitizeUrl(item.link),

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The main fix I need so I can, regression from v6

Comment thread src/utils.ts
export function sanitizeUrl(url: string | undefined): string | undefined {
export function sanitizeUrl(url: string | URL, xml?: boolean): string;
export function sanitizeUrl(url: string | URL | undefined, xml?: boolean): string | undefined;
export function sanitizeUrl(url: string | URL | undefined, xml = true): string | undefined {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

A default value of false for xml would be better, but it would require changing every call to RSS/Atom instead of only JSON. Please tell me if you indeed prefer the opposite!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant