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

Merge source maps with more clever algorithm #6

Closed
wants to merge 2 commits into from

Conversation

MotorHeat
Copy link

This will make normal debugging experience with TypeScript sources.

@tinchoz49
Copy link
Owner

Hi @MotorHeat ! thanks for the PR. I will be checking it in the next hours.

@MotorHeat
Copy link
Author

With TypeScript+Surplus we have this scheme:
tsc( file.ts ) --> (file.typescript.js, file.typescript.map )
surplus( file.typescript.js ) --> (file.surplus.js, file.surplus.map )
At the end we get 2 .map files (I put suffix .typescript and .susplus just to distinguish those files). And sources of those .map files are very different (so I assume the applySourceMap will not work properly here).
After the Surplus we get the .js file that will be executed by browser and the most correct source map for it. The only issue is that source map points to the output from TypeScript (file.typescript.js) instead of original file.ts file.
So we just need to fix that :) Idea of the algorithm is very simple:
Each generated line in file.surplus.map has "original line" from file.typescript.js, that "original line" is actually "generated line" in file.typescript.map, and we can find relevant "original line" in file.ts. If we are not able to find "original line" in file.ts then this means that line was generated by TypeScript and doesn't have representation in file.ts.
There is no need to use "find nearest line" - lines should be searched using "===" operator.
With very simplified way this can be written like that:

file.surplus.map.mapping[i].original.line = 
file.typescript.map.mapping.findByGeneratedLine( 
 file.surplus.map.mapping[i].generated ].original.line
).original.line

return result;
}

module.exports.mergeToPostProcessedSourceMap = mergeToPostProcessedSourceMap
Copy link
Owner

Choose a reason for hiding this comment

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

you can do: module.exports = mergeToPostProcessedSourceMap

Copy link
Owner

@tinchoz49 tinchoz49 left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation! it was really helpful to understand the PR.

So, two things about the PR:

  • The code seems ok but please can you run the new (I just added now): "npm run lint -- --fix" so the code can be standard compatible.
  • I think what you did here is something really helpful for all the surplus plugins out there. Did you think about creating a new module to put this functionality? So we can use it for all the plugins. (it's just an idea)

@MotorHeat
Copy link
Author

I think you are right that this thing should be in a separate npm module.
Also this algorithm is not ideal - it assumes that previous (to surplus) transpiler only adds new lines and don't joins them. In last case this algorithm will not work so good.
So may be I will think a little bit more on this, and may be applySourceMap from source-map package can work better after some errors will be fixed there.

@MotorHeat
Copy link
Author

I have played more deeply with sourceMap.SourceMapGenerator.applySourceMap function. It looks like it does what we exactly need here. The only issue is that this function calls sourceMap.SourceMapConsumer.originalPositionFor without specifying optional parameter bias.
If column position is not found, originalPositionFor returns nearest "left" (bias==GREATEST_LOWER_BOUND) or nearest right (bias == LEAST_UPPER_BOUND) element.
By default bias == GREATEST_LOWER_BOUND. At the same time Surplus and TypeScript produce column information in source maps differently for the same source line - Surplus include left white spaces, while TypeScript not.
So for Surplus case we should use bias == LEAST_UPPER_BOUND or make Surplus compiler do not include white spaces when generating source map.

I will abandon this PR because more simple solution exists.

@MotorHeat MotorHeat closed this Jan 20, 2019
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.

2 participants