Skip to content

Conversation

glowcloud
Copy link
Contributor

No description provided.

@glowcloud glowcloud self-assigned this Jun 27, 2025
@glowcloud glowcloud added enhancement New feature or request performance labels Jun 27, 2025
@glowcloud glowcloud marked this pull request as ready for review June 27, 2025 10:50
}

/* eslint-disable no-param-reassign */
private maybeAddSourceMap(node: unknown, element: Element): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

node: unknown - type with extends Node from apidom-ast package

*/
export const hasElementSourceMap = <T extends Element>(element: T): boolean => {
return isSourceMapElement(element.meta.get('sourceMap'));
return (
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably using a Number.isInteger is warranted here.

Copy link
Contributor

Choose a reason for hiding this comment

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

And let's make this function safe - meaning it never throws. We can use isElement and only continue after truthy assertion.

const sourceMapElement = element.getMetaProperty('sourceMap');
const charStart = toValue(sourceMapElement.positionStart.get(2));
const charEnd = toValue(sourceMapElement.positionEnd.get(2));
const charStart = element.startIndex;
Copy link
Contributor

Choose a reason for hiding this comment

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

Align naming with new sourcemap fields.

}
}
if (hasElementSourceMap(from)) {
to.startPositionRow = from.startPositionRow;
Copy link
Contributor

Choose a reason for hiding this comment

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

assignSourceMap(to, from)
->
to.startPositionRow = from.startPositionRow
...

Possibly making it safe later. No parameter destricturing as it might be used in syntactic analysis or refracting, so we want to keep it fast.

return new JsonDocument({
children: node.children,
position,
startPositionRow: node.startPositionRow,
Copy link
Contributor

Choose a reason for hiding this comment

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

return new JsonDocument(
  assignSourceMap({
    children: node.children,
    isMissing: node.isMissing,
  }, node)
);

Copy link
Contributor

@char0n char0n left a comment

Choose a reason for hiding this comment

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

Couple of comments to look at, but the overall direction is LEGIT

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants