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

Question: Surplus + TypeScript - debugging experience (issue with source maps?) #81

Open
MotorHeat opened this issue Jan 13, 2019 · 5 comments

Comments

@MotorHeat
Copy link

Hi,
First I want to say thank you for so great library (both Surplus and s-js).

I am trying to use Surplus together with TypeScript. And everything works fine except the debugging.
It is almost not possible to set breakpoints in the .ts and .tsx files.
It looks like the source maps are not properly updated.
With .js and .jsx files everything works fine.

This issue can be easily reproduced with original example in parcel-plugin-surplus.
Just try to put breakpoint in index.tsx file.

Do you have any suggestions how to improve/solve that?

@MotorHeat
Copy link
Author

There is nothing to do with this issue in surplus repository.
Issue is how bundlers merge source maps.
I have a solution for the parcel-plugin-surplus (look to this PR). Similar approach can be used in other bundlers.

So this question can be closed now.

@adamhaile
Copy link
Owner

That's an interesting PR! Yeah, this isn't just an issue with parcel but with all the build tools.

A friend of mine @TimHambourger looked into this a while back and found that it appeared to be caused by this issue in the source-map lib. Does your PR sound like it agrees with that? Still parsing through it.

@MotorHeat
Copy link
Author

I also initially look to source-map but then I realize that this library doesn't have anything relevant for Surplus case. There is method applySourceMap, which applies the "sub source map" - meaning of this term is steel not very clear for me. I get feeling that this method works for the case when the same source file is parsed several times by different tools, each of them produces source maps that should be merged (may be I am wrong here). But this is not the case for Surplus.
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

May be this is very specific for TypeScript+Surplus case and applySourceMap should work, but I don't think that you ever should use "find nearest" in this kind of source map merging, because you always have exact mapping between to sequentially generated source maps.

@MotorHeat
Copy link
Author

MotorHeat commented Jan 20, 2019

I have played more with applySourceMap and it looks like @TimHambourger is totally right.
The issue is that originalPositionFor returns null. The solution is to pass bias parameter to this function from applySourceMap was rejected I think.
The initial root cause is that Surplus and TypeScript produce column information in source maps differently for the same source line - Surplus include left white spaces, while TypeScript not.
If Surplus do not include leading spaces to source map then existing applySourceMap will work very good.

About "including white spaces to source map". Assume we have the following code:

1. let appState = {
2.     counter: S.data(0),
3.     inc: function() {
4.         appState.counter(appState.counter()+1)
5.     },
6.     dec: function() {
7.         appState.counter(appState.counter()-1)
8.     }
9. }

The value of "generatedColumn" of the line 2 of the above code sample in source map will be:
TypeScript: 4
Surplus: 0

If Surplus will skip leading spaces in this line then the resulting "generatedColumn" will be also 4 and originalPositionFor will try to search with generatedColumn == 4 in TypeScript mappings and will not return null.

Does it possible to implement in Surplus?
I will reopen this question because it seems like issue is in the way Surplus produces source maps.

@MotorHeat MotorHeat reopened this Jan 20, 2019
@MotorHeat
Copy link
Author

Here is the the commit to parcel-plugin-surplus with a little bit hacky solution to solve the mentioned above issue. I am not sure if it should be merged but you steel can test how it works by adding parcel-plugin-surplus like this:
yarn add motorheat/parcel-plugin-surplus#merge-source-map-original-position-for

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

2 participants