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.

Sign up for GitHub

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

Jump to bottom

Incorrect print of default export of template literal #616

Open
mheiber opened this issue Aug 1, 2019 · 1 comment · May be fixed by #619
Open

Incorrect print of default export of template literal #616

mheiber opened this issue Aug 1, 2019 · 1 comment · May be fixed by #619

Comments

@mheiber
Copy link

mheiber commented Aug 1, 2019 •
edited

When printing a default export whose value is or includes a template literal, there are extra backticks in the output:

const recast = require("recast");
const types = require("ast-types");

const ast = recast.parse("export default `${3}`");

types.visit(ast, {
    visitTemplateLiteral(path) {
        console.log(recast.print(path.node).code);
        return false;
    }
});

Actual behavior

The above example code logs '${3}', which isn't valid JS

Expected behavior

The above example code should log '${3}'

Details

Tested with Recast 0.18.1 (latest as of this writing)

Thanks for your help in figuring this out!

Deleting the two calls to 'parts.push("`")' in the case "TemplateLiteral" statement in printer.ts seems like it might fix the issue.
@mheiber
Copy link
Author

mheiber commented Aug 13, 2019

This bug is related to #556.
That code explicitly opts out of fixTemplateLiteral if the template literal is in a default export. So template literals won't be 'fixed' for default exports! Reverting that code wouldn't fix the underlying problem: instead, we would get a crash whenever someone does a recast.print of a default export with a template literal.

The .loc information for default exports was introduced during a fix for decorated default exports: 537a5ac

If I understand correctly, Recast loses .loc information on purpose, as a way of tricking the reprinter into not forgetting to print 'export default', per this comment

Some things I haven't figured out:

  1. Why is fixTemplateLiteral needed? In particular, why is fixTemplateLiteral only needed for recast.print, but isn't needed when doing recast.prettyPrint? My hunch is that Recast's abstraction over the underlying parser is off by one, so there is incorrect or misinterpreted information in the original AST Node's .loc. Is there an issue for fixing the root cause so we don't need fixTemplateLiteral?
  2. What do decorators have to do with this? Is there a non-tricky way of reprinting decorators correctly? Referring to this comment about nulling out .loc

mheiber added a commit to mheiber/recast that referenced this issue Aug 13, 2019
@mheiber
fix benjamn#616 print template literal in default export
9ca3aa4 
fix benjamn#616, which is a bug where

    export default `hello`

would print as the following (not valid JS):

    export default ``hello``

Fix by *not* nulling out .loc information
for the nodes, thus enabling them
to be processed by `fixTemplateLiteral`.

Related:
- benjamn#556 (comment)
- benjamn@537a5ac
mheiber added a commit to mheiber/recast that referenced this issue Aug 13, 2019
@mheiber
fix benjamn#616 print template literal in default export
efeb9f6 
fix benjamn#616, which is a bug where

    export default `hello`

would print as the following (not valid JS):

    export default ``hello``

Fix by *not* nulling out .loc information
for the nodes, thus enabling them
to be processed by `fixTemplateLiteral`.

Related:
- benjamn#556 (comment)
- benjamn@537a5ac
mheiber added a commit to mheiber/recast that referenced this issue Aug 13, 2019
@mheiber
fix benjamn#616 print template literal in default export
aa03c3c 
fix benjamn#616, which is a bug where

    export default `hello`

would print as the following (not valid JS):

    export default ``hello``

Fix by *not* nulling out .loc information
for the nodes, thus enabling them
to be processed by `fixTemplateLiteral`.

Related:
- benjamn#556 (comment)
- benjamn@537a5ac
@mheiber mheiber linked a pull request Aug 13, 2019 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix #616 print template literal in default export mheiber/recast
1 participant
@mheiber