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

AST location information lost when emitting yield replacement (and others) #342

Closed
loganfsmyth opened this issue Feb 16, 2018 · 2 comments

Comments

Projects
None yet
2 participants
@loganfsmyth
Copy link
Contributor

commented Feb 16, 2018

See in this sample from http://sokra.github.io/source-map-visualization for a sample async function passed through Babel.
screen shot 2018-02-16 at 10 46 43 am

Note how the return before doAsync is blue instead of white, and the right side shows no generated mapping for the await.

This specific example is for

self.emit(t.returnStatement(arg || null));

Where essentially ideally it would do

let ret = t.returnStatement(arg || null);
ret.loc = expr.loc;
self.emit(ret);

so the location information from the yield expression is carried over onto the return statement.

Depending how ambitious we want to be, this probably also comes up in a few other places where regenerator creates new AST nodes, like all of these cases:

return finish(t.updateExpression(

the replacement UpdateExpression will not have the location information that the original node did, which may not be ideal.

Think I've got someone who wants to look into making a PR for this. Handling the return at least would be ideal, but it might be good to account for the other cases too.

@benjamn

This comment has been minimized.

Copy link
Collaborator

commented Feb 16, 2018

This seems doable! In general, statements that do not contain any yield expressions get left alone, so their .loc information should be intact, but I would imagine any newly created AST nodes will be missing .loc information. If preserving the .loc property is all that has to happen for source maps to be useful within async and/or generator functions, that's great news.

@loganfsmyth

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2018

Yeah, I think it at least partially depends on how ambitious someone wants to be with fixing things. This could be split into multiple tasks. In this case at least fixing the return allows users to who are stepping through code to step onto an await expression statement properly, because when they step to the return it will map back to await.

Right now you can set a breakpoint on await doAsync() and it will take the closes point and stop at doAsync(), but you can't actually reach it stepping one statement at a time because it steps to the return and the return doesn't map to a useful location. At least in FF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.