Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve call expression source maps #60565

Open
mini-ninja-64 opened this issue Nov 22, 2024 · 0 comments
Open

Improve call expression source maps #60565

mini-ninja-64 opened this issue Nov 22, 2024 · 0 comments

Comments

@mini-ninja-64
Copy link

mini-ninja-64 commented Nov 22, 2024

πŸ”Ž Search Terms

  • source map call expression
  • emitCallExpression source map
  • source map function call

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about "Source Maps"

⏯ Playground Link

https://www.typescriptlang.org/play/?module=1&inlineSourceMap=true#code/JYWwDg9gTgLgBCCBjA1gUQB4wKZQHYCGANnAGZQQhwBEAdAPSKq0BWAztQNwBQ3pArniQxgEPAmQoAPABUAfAAoCUAOYB+AFxwZASgDeAX15IxbeATgBeCagU6eJvGbgAjKzZRwFcOPe6PnJHcmaTMoYDwVOR8FagJ4gmo-APgAE2DJOHoAKgRsCAB3OGz6Lx8aZRUARmpyv256UoA5CBwtUmgkbDgYNiCYCDgVbDxcAhw4AnEI1OAobGE4JGIif1N4busQzBx8YnKymlrfHiA

πŸ’» Code

import mockExternal from "./mock.js";

function mock<T>(arg?: T){}

const a = mock();
const b = mock (  );
const c = mock<string>  ("aaaa");
const d = mock /* meow */ (   "arg1"   );

// Note: force tsc to generate an indirect call
const e = mockExternal    (  ""  );

Compiled:

"use strict";
var __importDefault = (this && this.__importDefault) || function (mod) {
    return (mod && mod.__esModule) ? mod : { "default": mod };
};
Object.defineProperty(exports, "__esModule", { value: true });
const mock_js_1 = __importDefault(require("./mock.js"));
function mock(arg) { }
const a = mock();
const b = mock();
const c = mock("aaaa");
const d = mock /* meow */("arg1");
// Note: force tsc to generate an indirect call
const e = (0, mock_js_1.default)("");
//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiaW5wdXQuanMiLCJzb3VyY2VSb290IjoiIiwic291cmNlcyI6WyJpbnB1dC50c3giXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6Ijs7Ozs7QUFBQSx3REFBcUM7QUFFckMsU0FBUyxJQUFJLENBQUksR0FBTyxJQUFFLENBQUM7QUFFM0IsTUFBTSxDQUFDLEdBQUcsSUFBSSxFQUFFLENBQUM7QUFDakIsTUFBTSxDQUFDLEdBQUcsSUFBSSxFQUFLLENBQUM7QUFDcEIsTUFBTSxDQUFDLEdBQUcsSUFBSSxDQUFXLE1BQU0sQ0FBQyxDQUFDO0FBQ2pDLE1BQU0sQ0FBQyxHQUFHLElBQUksQ0FBQyxVQUFVLENBQUssTUFBTSxDQUFJLENBQUM7QUFFekMsK0NBQStDO0FBQy9DLE1BQU0sQ0FBQyxHQUFHLElBQUEsaUJBQVksRUFBTyxFQUFFLENBQUcsQ0FBQyJ9

πŸ™ Actual behavior

The JS is compiled perfectly and works fine, however due to the way NodeJS reports call sites the call site of each mock function invocation is as follows:

position in JS (line, col) substring in JS at position position after source mapping substring in TS at position
(8, 11) mock (5, 11) mock
(9, 11) mock (6, 11) mock
(10, 11) mock (7, 11) mock
(11, 11) mock (8, 11) mock
(13, 31) (""); (11, 29) "" );

When looking at a call expressions reported call site I personally think that indirect call expressions positions are misleading after being source mapped as it seems to imply the first argument is the section of the code affected when there are spaces between the first argument and the open parenthesis token.

πŸ™‚ Expected behavior

I would expect the (13,31) call site to map directly to (12, 27) in the source map, so that users can easily see the invocation position of the function call, in a consistent and predictable way.

Additional information about the issue

I debated on whether this would be considered a bug or a feature request, but decided on bug due to the current call site position being inconsistent for users trying to map their codes call location. If you think this should be a feature request, I am more than happy to change this issue to be a feature request.

If this feature is deemed to be an acceptable change to typescripts source map emissions, I have created a simple implementation that in my own testing seems to work, I am happy to add automated tests and raise a PR (https://github.com/mini-ninja-64/TypeScript/tree/improve-source-map). My current implementation works by emitting a source map position in the emitNodeList function based on the start position of the provided NodeArray<T>, this means it could also improve the source map production for other node lists; however this could increase the size of source maps generated by tsc.

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

No branches or pull requests

1 participant