babel/babel

`BatchRenamer`, 100x faster `scope.rename` when you have many identifiers to rename

remorses opened this issue · 6 comments

💻

  • Would you like to work on this feature?

What problem are you trying to solve?

scope.rename is slow when you want to rename many identifiers, the complexity grows with the number of identifiers

Describe the solution you'd like

BatchRenamer, an utility to rename many identifiers in one traversal

const renamer = new BatchRenamer(
    path.scope,
    new Map(['old', 'new'])
)
renamer.rename()

// or attach a method directly on scope
scope.batchRename(new Map(['old', 'new']))

Describe alternatives you've considered

call scope.rename N times, time complexity grows with N, you make N traversals of the full AST

Documentation, Adoption, Migration Strategy

I have already written an implementation i am already using, it adds minimal overhead compared to the already existing Renamer, in my case i had 100 identifiers to rename and it becomes 100x faster.

I am not familiar with the Babel codebase, if someone wants to open a PR to add the code feel free to do it

https://gist.github.com/remorses/9a11d96f9f00d3af1388a197be2a7878

Hey @remorses! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

Thanks for this!

Maybe we could make rename accept an array of string | string[]?

In my code I used a Map that goes from old names to new names. I am not sure if this feature would be used enough and if it would make sense to add to babel core

We would at least use it in the commonjs transform, where we rename multiple identifiers one after the other, and probably in the let/const transform where we can batch-rename variables when converting them to var.

Why is a full traverse needed in the first place? Got a massive performance boost by looping over binding.referencePaths/constantViolations instead (without batching)

Why is a full traverse needed in the first place? Got a massive performance boost by looping over binding.referencePaths/constantViolations instead (without batching)

Because binding.referencePaths/constantViolations may be inaccurate, using them may have some risks.