Skip to content

Comments

fix: readme rendering result lacks anchor point jump#994

Merged
danielroe merged 1 commit intonpmx-dev:mainfrom
btea:fix/readme-lost-tracing-point-jump
Feb 5, 2026
Merged

fix: readme rendering result lacks anchor point jump#994
danielroe merged 1 commit intonpmx-dev:mainfrom
btea:fix/readme-lost-tracing-point-jump

Conversation

@btea
Copy link
Contributor

@btea btea commented Feb 5, 2026

fix #989

@vercel
Copy link

vercel bot commented Feb 5, 2026

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

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 5, 2026 7:32am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 5, 2026 7:32am
npmx-lunaria Ignored Ignored Feb 5, 2026 7:32am

Request Review

@codecov
Copy link

codecov bot commented Feb 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

@danielroe danielroe added this pull request to the merge queue Feb 5, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 5, 2026

📝 Walkthrough

Walkthrough

The changes modify HTML sanitisation in the readme rendering utility. A wildcard key '*' is introduced in ALLOWED_ATTR to enable 'id' attributes across all HTML tags, removing redundant per-tag declarations. A helper function prefixes id attributes with 'user-content-' during sanitisation, applied via the transformTags pipeline to block elements including div, p, span, section, and article. Public function signatures remain unchanged; modifications are internal to the README HTML rendering and sanitisation logic. The adjustment involves 31 additions and 17 deletions.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The pull request description 'fix #989' is minimal but directly references the linked issue, making it related to the changeset.
Linked Issues check ✅ Passed The changes correctly implement anchor-point functionality by prefixing IDs with 'user-content-' as shown in the issue's URL, enabling hash navigation to README sections.
Out of Scope Changes check ✅ Passed All modifications are contained within server/utils/readme.ts and directly address the hash URL navigation issue without introducing unrelated changes.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Comment on lines +401 to +407
// Helper to prefix id attributes with 'user-content-'
const prefixId = (tagName: string, attribs: sanitizeHtml.Attributes) => {
if (attribs.id && !attribs.id.startsWith('user-content-')) {
attribs.id = `user-content-${attribs.id}`
}
return { tagName, attribs }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Make fragment normalisation idempotent to avoid double‑prefixing.

Links already using #user-content-… (common when copied from GitHub) will become #user-content-user-content-… because resolveUrl always prefixes hash fragments. Guard against that so those anchors still resolve.

🔧 Proposed fix
function resolveUrl(url: string, packageName: string, repoInfo?: RepositoryInfo): string {
  if (!url) return url
  if (url.startsWith('#')) {
    // Prefix anchor links to match heading IDs (avoids collision with page IDs)
-    return `#user-content-${url.slice(1)}`
+    if (url.startsWith('#user-content-')) return url
+    return `#user-content-${url.slice(1)}`
  }

Comment on lines +448 to +452
div: prefixId,
p: prefixId,
span: prefixId,
section: prefixId,
article: prefixId,
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's locate the file and check its existence
find . -name "readme.ts" -path "*/server/utils/*" 2>/dev/null | head -20

Repository: npmx-dev/npmx.dev

Length of output: 85


🏁 Script executed:

# Check the file size and read the specific lines mentioned
if [ -f "server/utils/readme.ts" ]; then
  wc -l server/utils/readme.ts
  echo "=== Lines 440-460 ==="
  sed -n '440,460p' server/utils/readme.ts
fi

Repository: npmx-dev/npmx.dev

Length of output: 648


🏁 Script executed:

# Search for the transformTags configuration to understand the full context
rg "transformTags" server/utils/readme.ts -A 30 -B 5

Repository: npmx-dev/npmx.dev

Length of output: 1470


🏁 Script executed:

# Look for the prefixId function definition
rg "prefixId\s*[:=]|function prefixId" server/utils/readme.ts -B 2 -A 10

Repository: npmx-dev/npmx.dev

Length of output: 511


🏁 Script executed:

# Check package.json for sanitize-html version
cat package.json | grep -A 2 -B 2 "sanitize-html"

Repository: npmx-dev/npmx.dev

Length of output: 377


🌐 Web query:

sanitize-html 2.17.0 wildcard transform support transformTags

💡 Result:

In sanitize-html v2.17.0, transformTags supports a '*' wildcard key to apply a transform to all tags (not just a specific tag name). [1][2]

import sanitizeHtml from "sanitize-html";

const clean = sanitizeHtml(dirtyHtml, {
  transformTags: {
    "*": (tagName, attribs) => ({
      tagName,
      attribs: { ...attribs, "data-sanitized": "1" }
    })
  }
});

You can also use the wildcard with sanitizeHtml.simpleTransform(...) for simpler “add/override attributes” cases. [1][2]


🏁 Script executed:

# Also search for any usage of wildcard transforms in the codebase
rg "transformTags" . -A 50 | grep -E "(\*|wildcard)" || echo "No wildcard patterns found in transformTags"

Repository: npmx-dev/npmx.dev

Length of output: 104


🏁 Script executed:

# Let's check if there are any fragment link rewrites happening elsewhere
rg "user-content" server/utils/readme.ts -B 3 -A 3

Repository: npmx-dev/npmx.dev

Length of output: 1054


🏁 Script executed:

# Check the ALLOWED_TAGS to see what tags are allowed through
rg "ALLOWED_TAGS\s*=" server/utils/readme.ts -A 30

Repository: npmx-dev/npmx.dev

Length of output: 366


Prefix IDs for anchor elements to match rewritten fragment links.

Fragment links are rewritten to #user-content-foo, but anchor IDs are not prefixed, creating a mismatch. Apply prefixId to the a transform to keep anchor jumps consistent.

♻️ Proposed fix (apply to `a`)
      a: (tagName, attribs) => {
        // Add security attributes for external links
        if (attribs.href && hasProtocol(attribs.href, { acceptRelative: true })) {
          attribs.rel = 'nofollow noreferrer noopener'
          attribs.target = '_blank'
        }
-        return { tagName, attribs }
+        return prefixId(tagName, attribs)
      },

Alternatively, sanitize-html 2.17.0 supports a wildcard '*' transform in transformTags if you prefer centralising ID prefixing.

Merged via the queue into npmx-dev:main with commit 0531e3f Feb 5, 2026
16 checks passed
@btea btea deleted the fix/readme-lost-tracing-point-jump branch February 5, 2026 07:41
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.

bug: hash url doesnt work

2 participants