Skip to content

fix(error-messages): improve hints in xref, mdn-annotation, link-to-dfn#5135

Open
marcoscaceres wants to merge 3 commits intomainfrom
fix/error-message-hints
Open

fix(error-messages): improve hints in xref, mdn-annotation, link-to-dfn#5135
marcoscaceres wants to merge 3 commits intomainfrom
fix/error-message-hints

Conversation

@marcoscaceres
Copy link
Copy Markdown
Contributor

Summary

Improves three error/warning messages that were either missing hints or had hints too vague to act on. Found during an error message quality audit across ~85 showError/showWarning calls.

src/core/xref.js

Invalid xref config value had no hint at all. Now explains all four valid forms:

Expected: `true`, a profile name (e.g. `"web-platform"`), an array of spec shortnames
(e.g. `["FETCH", "DOM"]`), or an object with `url`, `specs`, or `profile` properties.
See [xref].

src/core/mdn-annotation.js

Hint said "Please add a valid key to respecConfig.mdn" but gave no indication of where to find valid keys. Now links directly to the source of truth:

Could not find MDN data for key "X". Browse available keys at
https://w3c.github.io/mdn-spec-links/SPECMAP.json and set respecConfig.mdn
to the matching key.

src/core/link-to-dfn.js

Linkless <a> warning had no hint. Authors seeing "no matching <dfn>" had no guidance on what to do. Now suggests three actionable paths:

Add a matching [<dfn>] element, use [data-cite] to link to an external definition,
or enable [xref] for automatic cross-spec linking.

Also adds docLink to the imports in link-to-dfn.js to enable the linked hint text.

🤖 Generated with Claude Code

@marcoscaceres marcoscaceres force-pushed the fix/error-message-hints branch 4 times, most recently from f402eba to a2b73df Compare March 28, 2026 11:46
xref.js: invalid xref config value had no hint. Now explains the four
valid forms: boolean, profile name, array of shortnames, or object.

mdn-annotation.js: hint said 'add a valid key' but didn't say where to
find one. Now links directly to SPECMAP.json.

link-to-dfn.js: linkless <a> warning had no hint. Now suggests adding
a <dfn>, using data-cite, or enabling xref. Also adds docLink import.

docs: add error message audit report (2026-03-28-error-message-audit.md)
documenting all 8 issues found, 6 already fixed, patterns to follow.
Copy link
Copy Markdown

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

Improves author-facing lint/error hints in core modules to make configuration and linking failures more actionable and easier to resolve.

Changes:

  • Add a detailed xref config hint (with docs link) for invalid configuration types.
  • Improve the MDN annotation missing-key hint by pointing to the MDN key registry.
  • Add actionable guidance (and import docLink) for linkless <a> warnings in link-to-dfn.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/core/xref.js Adds a docLink-backed hint describing valid xref configuration forms.
src/core/mdn-annotation.js Updates the 404 hint text to point users at the MDN spec map URL.
src/core/link-to-dfn.js Adds docLink import and expands the warning hint for linkless anchors, including a scoping note.

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

@@ -312,7 +313,14 @@ function showLinkingError(elems) {
elems.forEach(elem => {
const msg = `Found linkless \`<a>\` element with text "${elem.textContent}" but no matching \`<dfn>\``;
const title = "Linking error: not matching `<dfn>`";
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The warning title reads "not matching <dfn>", which is grammatically off and inconsistent with the message (“no matching <dfn>”). Consider changing it to “no matching <dfn>”.

Suggested change
const title = "Linking error: not matching `<dfn>`";
const title = "Linking error: no matching `<dfn>`";

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed title in 4a94f64 ('not matching' → 'no matching'). Also corrected the scoping note — data-dfn-for on an ancestor does NOT scope link resolution (that's data-link-for), so the old message was misleading. Kept the detection heuristic using data-dfn-for since that's still the right context to flag; just fixed what the message says.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 4a94f64.

if (res.status === 404) {
const msg = `Could not find MDN data associated with key "${key}".`;
const hint = "Please add a valid key to `respecConfig.mdn`";
const hint = `Could not find MDN data for key "${key}". Browse available keys at https://w3c.github.io/mdn-spec-links/SPECMAP.json and set respecConfig.mdn to the matching key.`;
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

This hint suggests setting respecConfig.mdn, but the config also supports an object form (e.g., respecConfig.mdn.key). To avoid confusing users who already use the object form, consider mentioning both, and format the config keys/URL as markdown (e.g., backticks and a clickable link).

Suggested change
const hint = `Could not find MDN data for key "${key}". Browse available keys at https://w3c.github.io/mdn-spec-links/SPECMAP.json and set respecConfig.mdn to the matching key.`;
const hint = `Could not find MDN data for key "${key}". Browse available keys at [\`SPECMAP.json\`](https://w3c.github.io/mdn-spec-links/SPECMAP.json) and set \`respecConfig.mdn\` or \`respecConfig.mdn.key\` to the matching key.`;

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This hint is superseded by PR #5137 which rewrites it with docLink syntax, mentions both mdn: true and mdn.key, and links directly to SPECMAP.json. No change here to avoid conflicts.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Superseded by PR #5137 which rewrites this hint with docLink, mentions both mdn: true and mdn.key, and links to SPECMAP.json.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, the hint is repeating title here. When both are shown together, it's just noise.

- Fix title: 'not matching' → 'no matching' (consistent with msg text)
- Fix hint: data-dfn-for does NOT scope link resolution (data-link-for
  does). Old message incorrectly said links were scoped to the context.
  New message correctly explains the distinction and shows how to
  explicitly scope a link with [=Iface/term=] or data-link-for.
Copy link
Copy Markdown

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


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

Copy link
Copy Markdown

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.


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

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.

3 participants