Skip to content

Conversation

lumirlumir
Copy link
Member

@lumirlumir lumirlumir commented Oct 1, 2025

Prerequisites checklist

What is the purpose of this pull request?

Which language are you using?

CommonMark and GFM.

What did you do?

I expected the return type of applyInlineConfig().configs — specifically loc.start and loc.end — to include an offset property, but they don't.

What did you expect to happen?

I expect the types of loc.start and loc.end to include an offset property.

Link to minimal reproducible Example

import { MarkdownSourceCode } from "./src/index.js";

const markdownSourceCode = new MarkdownSourceCode({
	text: '<!-- eslint markdown/no-html: "error" -->',
	ast: {
		type: "root",
		children: [
			{
				type: "html",
				value: '<!-- eslint markdown/no-html: "error" -->',
				position: {
					start: {
						line: 1,
						column: 1,
						offset: 0,
					},
					end: {
						line: 1,
						column: 42,
						offset: 41,
					},
				},
			},
		],
		position: {
			start: {
				line: 1,
				column: 1,
				offset: 0,
			},
			end: {
				line: 1,
				column: 42,
				offset: 41,
			},
		},
	},
});

console.log(markdownSourceCode.applyInlineConfig().configs[0].loc);

If you run the code above, you'll see that markdownSourceCode.applyInlineConfig().configs[0].loc.start and markdownSourceCode.applyInlineConfig().configs[0].loc.end include an offset property. However, the return type doesn't reflect that offset property.

# Output
{
  start: { line: 1, column: 1, offset: 0 },
  end: { line: 1, column: 42, offset: 41 }
}

What changes did you make? (Give an overview)

Looking at the origin of the loc property returned by applyInlineConfig, you'll find that it directly uses the position property of the Html node.

const start = {
...node.position.start,
};

start.offset += match.index;

const end = {
...node.position.start,
};

end.offset = start.offset + comment.length;

position: {
start,
end,
},

The position property uses the Position type from unist as shown below, so technically it doesn’t match the SourceLocation type.

  • Position
interface Position {
    start: {
        line: number;
        column: number;
        offset?: number | undefined;
    }
    end: {
        line: number;
        column: number;
        offset?: number | undefined;
    }
}
  • SourceLocation
interface SourceLocation {
    start: {
        line: number;
        column: number;
    }
    end: {
        line: number;
        column: number;
    }
}

To provide a more accurate return type, I've replaced the SourceLocation type with the Position type from unist.

Related Issues

Ref: eslint/css#281, eslint/json#162

Is there anything you'd like reviewers to focus on?

N/A

@eslint-github-bot eslint-github-bot bot added the bug label Oct 1, 2025
@eslintbot eslintbot added this to Triage Oct 1, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in Triage Oct 1, 2025
@fasttime
Copy link
Member

fasttime commented Oct 6, 2025

I can reproduce this. Marking as accepted.

@fasttime fasttime moved this from Triaging to Implementing in Triage Oct 6, 2025
Copy link
Member

@fasttime fasttime left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@fasttime fasttime merged commit d6621a7 into main Oct 6, 2025
25 of 26 checks passed
@fasttime fasttime deleted the fix-correct-the-return-type-of-applyinlineconfig branch October 6, 2025 10:32
@github-project-automation github-project-automation bot moved this from Implementing to Complete in Triage Oct 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants