microsoft / microsoft/TypeScript
Incorrect Source Map Generation
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 111k
- Forks
- 14.3k
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 132
Description
TypeScript Version: 2.5.2
Search Terms: source map
Code
This is sadly kind of hard to show.
example.ts
import * as fs from 'fs';
import { message } from './extra';
console.log(message, fs.readFileSync(__FILE__));
extra.ts (force imports to get mangled by tsc)
export const message = 'example:';
tsc example.ts --sourceMap
(you'll get some errors due to minimal example not having nodejs types, they don't matter)
The resulting files:
example.js
"use strict";
exports.__esModule = true;
var fs = require("fs");
var extra_1 = require("./extra");
console.log(extra_1.message, fs.readFileSync(__FILE__));
//# sourceMappingURL=example.js.map
example.js.map
{"version":3,"file":"example.js","sourceRoot":"","sources":["example.ts"],"names":[],"mappings":";;AAAA,uBAAyB;AACzB,iCAAkC;AAClC,OAAO,CAAC,GAAG,CAAC,eAAO,EAAE,EAAE,CAAC,YAAY,CAAC,QAAQ,CAAC,CAAC,CAAC"}
You can use the following tool to visualize the mapping https://sokra.github.io/source-map-visualization/
Expected behavior:
The "extra_1.message" should have 3 columns specified in the source map. One for "extra_1", one for "." and another for "message"
Actual behavior:
Only the column for "extra_1" is present (likely because it's just translating from the original source map?)
This is a problem because browser/node stack traces will ask for the mapping for said columns. So for example in nodejs you may get in cases the column for "." as the source column, not the column for "extra_1". This is possible to mitigate, but ideally should not be needed to. Any difference of more then 1 position is ambigous on how bad the source map error is.
Related Issues:
Imports are very noisy and get unnecesarily mangled
Fixing this would indirectly solve almost all cases where the issue matters, since traces are unlikely to occur on the import lines, and the following would remove those cases so translating is 1:1 (in case enhancing the map is too hard)
Take import { message } from './extra'; from above as an example. It gets converted to var extra_1 = require("./extra"); and extra_1.message. This seems like pointlessly defensive code.
I'm sure this is optimized at runtime, so it's not a perf issue necesarily, though still very noisy. It would be much better to if it was var message = require("./extra").message, then usage would stay as message as per the original source. (Note: imports also suffer from the source map issue, they are a single column + one terminal unknown "thing"[?], instead of 6-7).
For multi-line imports destruction or multi-line assignment would be better as well.
eg.
var __imports = {};
__imports.extra = require('./extra');
var message = imports.extra;
var title = imports.extra;
Since the names message and title would have already been used in the source they are practically like keywords, there's no risk of unintended shadowing outside of complex structures generated by typescript itself; same can not necessarily be said of things such as "extra_1". More noise at the top is much preferred over noise in the actual code.
Whitespace should not be removed
Since files are not minified it would be better if empty lines were maintained.
If there is any concern with size (bytes/s while reading) because of whitespace then actually using tabs instead of spaces would yield greater gain, over not removing newlines (\n)
Names are ignored
Symbol names should probably included. If not by default at least with a flag.
I can understand them not being included to save space.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Reproduce the example with example.ts and extra.ts using tsc example.ts --sourceMap, then inspect example.js.map with the linked source-map visualization tool. Compare the generated mapping for extra_1.message with the expected mappings for extra_1, ., and message; the issue does not name compiler source files or tests to update.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100