Skip to content

Conversation

@ofersadgat
Copy link

@ofersadgat ofersadgat commented Dec 10, 2024

Fixes: #1591

Note that using .pos matches what typescript is doing as well:

function visitNodes2(nodes, visitor, test, start, count) {
  if (nodes === void 0) {
    return nodes;
  }
  const length2 = nodes.length;
  if (start === void 0 || start < 0) {
    start = 0;
  }
  if (count === void 0 || count > length2 - start) {
    count = length2 - start;
  }
  let hasTrailingComma;
  let pos = -1;
  let end = -1;
  if (start > 0 || count < length2) {
    hasTrailingComma = nodes.hasTrailingComma && start + count === length2;
  } else {
    pos = nodes.pos;
    end = nodes.end;
    hasTrailingComma = nodes.hasTrailingComma;
  }
  const updated = visitArrayWorker(nodes, visitor, test, start, count); 
  if (updated !== nodes) {
    const updatedArray = factory.createNodeArray(updated, hasTrailingComma);
    setTextRangePosEnd(updatedArray, pos, end);
    return updatedArray;
  }
  return nodes;
}

The line:
const updated = visitArrayWorker(nodes, visitor, test, start, count);
is calling visitArrayWorker which calls innerVisit which first calls the transform function followed by the handleTransformation function. So, on the way out typescript is using .pos as well.

@dsherret
Copy link
Owner

I'm not sure this is correct and it seems to cause some tests to fail.

@ofersadgat ofersadgat reopened this Nov 6, 2025
@ofersadgat
Copy link
Author

@dsherret Hi, I have done some more due diligence here. The code IS incorrect for a specific circumstance: JsxText. The reason for this is that getStart(true) includes comments but excludes leading whitespace. The problem, as you've mentioned elsewhere, is that leading whitespace is actually a part of a JsxText node. Therefore, excluding that when you're doing a transform is incorrect. This leads to the result of continually prepending whitespace to the transformed jsx node.

You run into a similar situation with replaceWithText(). This function similarly also doesnt include leading whitespace. This leads to the problem:

const text = node.getFullText();
node.replaceWithText(text);

this will also prepend whitespace which is counter intuitive. Also, afaict there isnt an api around this. So I have added an option into the replaceWithText api to allow replacing the full thing. If there is a prefered alternative change, lmk.

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.

ts-morph incorrectly transforms nodes which begin with whitespace

2 participants