Skip to content

Conversation

@efekrskl
Copy link
Contributor

@efekrskl efekrskl commented Nov 10, 2025

Description

Fixes a minor issue where article headings could have duplicate keys, when they share the same title (e.g https://nodejs.org/en/learn/test-runner/mocking, https://nodejs.org/en/learn/diagnostics/memory)

image

Validation

Related Issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run pnpm format to ensure the code follows the style guide.
  • I have run pnpm test to check if all tests are passing.
  • I have run pnpm build to check if the website builds without errors.
  • I've covered new added functionality with unit tests if necessary.

@efekrskl efekrskl requested a review from a team as a code owner November 10, 2025 20:18
Copilot AI review requested due to automatic review settings November 10, 2025 20:18
@vercel
Copy link

vercel bot commented Nov 10, 2025

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

Project Deployment Preview Updated (UTC)
nodejs-org Ready Ready Preview Nov 11, 2025 6:41pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a React duplicate key warning issue that occurred when article headings share the same text content. The fix changes the list key from the heading text (head.value) to the unique heading ID (head.data?.id) generated by the rehype-slug plugin.

  • Changed the React key for heading list items from text-based to ID-based to ensure uniqueness
  • Improved consistency in optional chaining syntax for the href attribute

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Nov 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.76%. Comparing base (ca6a3dd) to head (69c283b).
⚠️ Report is 9 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8348   +/-   ##
=======================================
  Coverage   76.76%   76.76%           
=======================================
  Files         118      118           
  Lines        9822     9822           
  Branches      335      335           
=======================================
  Hits         7540     7540           
  Misses       2280     2280           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bjohansebas
Copy link
Member

bjohansebas commented Nov 11, 2025

@efekrskl you can do a rebase or make an empty commit so Vercel triggers a new preview, please?. I’m not sure why it failed on Vercel, but it seems that everything is fine in the GitHub action. If the error keeps happening, let us know on Slack

@efekrskl
Copy link
Contributor Author

@efekrskl you can do a rebase or make an empty commit so Vercel triggers a new preview, please?. I’m not sure why it failed on Vercel, but it seems that everything is fine in the GitHub action. If the error keeps happening, let us know on Slack

Yup, seems to be working fine now

@avivkeller avivkeller added this pull request to the merge queue Nov 12, 2025
Merged via the queue into nodejs:main with commit 4624ae9 Nov 12, 2025
12 checks passed
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.

4 participants