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

Critical assertion error generating source maps #831

Closed
GianlucaGuarini opened this issue Dec 23, 2020 · 7 comments
Closed

Critical assertion error generating source maps #831

GianlucaGuarini opened this issue Dec 23, 2020 · 7 comments

Comments

@GianlucaGuarini
Copy link

GianlucaGuarini commented Dec 23, 2020

I am working since days on a Riot.js compiler bug, this issue is driving me mad. I have noticed that the problem is coming from the recast.print method when it tries to generate the sourcemaps.

I was able to reproduce the issue with a few lines of code here

That's the code to reproduce it in case you want to try to copy and paste it directly in your editor:

const recast = require('recast')
const types = recast.types
const builders = recast.types.builders
const scope = builders.identifier('scope')

const expression = `(




                                                 {
    complete: onComplete,
    translateX: 100
  })`

const ast = recast.parse(expression, {
  sourceFileName: 'test.js',

})

// replace the scope of the identifiers found
types.visit(ast, {
  visitIdentifier(path) {
    path.replace(builders.memberExpression(
      scope,
      path.node,
      false
    ))

    return false
  }
})

// crate a function 
const result = builders.functionExpression(
  null,
  [scope],
  builders.blockStatement([builders.returnStatement(
    ast.program.body[0].expression
  )])
)

// generate the output
const output = recast.print(result, {
  sourceMapName: `source.map`
})

return output

Any hint or help solving this issue is really appreciated as long as the sourcemap output will be correct :)

Thank you

@eventualbuddha
Copy link
Collaborator

Replacing an identifier with a member expression is not always safe. You’ll be replacing the keys in your object with e.g. scope.translateX, which is not a valid key.

@GianlucaGuarini
Copy link
Author

@eventualbuddha thank you for your feedback. The @riotjs/compiler is much more complex than this demo. The code above is just needed to show the issue in recast that seems to be clearly related to the breaking spaces since the following demo works as expected: https://runkit.com/gianlucaguarini/5fe44ae13e9cdb001a56fc21

@eventualbuddha
Copy link
Collaborator

Ah, you're right. I don't know what's causing the issue you're seeing, but it does appear to be a bug. Thanks for clarifying.

@GianlucaGuarini
Copy link
Author

GianlucaGuarini commented Oct 3, 2021

@benjamn do you accept PRs? I might be interested in solving this issue with a patch

@eventualbuddha
Copy link
Collaborator

Yep, though it may not happen quickly.

GianlucaGuarini added a commit to GianlucaGuarini/recast that referenced this issue Oct 3, 2021
@GianlucaGuarini
Copy link
Author

@eventualbuddha I have tried to keep the patch simple #989 but your help on it is highly appreciated.

@GianlucaGuarini
Copy link
Author

I am not able to reproduce this issue with the newest recast version. I am going to close this issue and my PR.
Thank you anyway :)

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 a pull request may close this issue.

2 participants