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

Beautified sourcemaps are sometimes incorrect #371

Closed
connor4312 opened this issue Mar 1, 2020 · 4 comments
Closed

Beautified sourcemaps are sometimes incorrect #371

connor4312 opened this issue Mar 1, 2020 · 4 comments
Labels
debt Code quality issues
Milestone

Comments

@connor4312
Copy link
Member

connor4312 commented Mar 1, 2020

js-beautify doesn't support sourcemaps. It's all handrolled, so very unlikely to change. I did some investigation into possible alternatives. esprima+escodegen are okay, not very fast however (todo: compare to js-beautify+our manual sourcemap generator). swc, a fancy Rust tool is actually slower (swc-project/swc#695). In my investigation, using @babel/generator and @babel/parser are the fastest pair of tools. We'll just need to do some work with the AST to unminify compacted expressions, which should not be overly difficult.

@connor4312 connor4312 added the debt Code quality issues label Mar 1, 2020
@connor4312 connor4312 added this to the March 2020 milestone Mar 1, 2020
@connor4312
Copy link
Member Author

I looked into this a bit and have a babel setup that partially works in https://github.com/microsoft/vscode-js-debug/tree/feat/babel-unminify. However, to beautify in a really nice way, we want to make structural changes to the code, such as swapping sequence expressions for separate statements. Doing this also means we need to step over the code differently--while the user sees a statement they want to step over, we need to step over one subexpression in the sequence. This is possible to do, but will take a good amount of work that I don't want to tackle at this time. Moving to backlog.

@connor4312
Copy link
Member Author

connor4312 commented Aug 10, 2020

Following up -- currently js-debug does not drive any debug behavior from source contents, and I would like to avoid doing so. Therefore a simple formatting of the AST is required, and structural changes must not happen. It appears that js-beautify can sometimes alter the AST, as in the linked issue.

I want to look into grabbing swc, if its compiled file size is good (they've since fixed the performance issue which made it slow). If that is impractical due to size or other constraints, babel is a good backup. We can alternatively hand-format the TS AST, since we don't really need source-quality formatting, but again I'd rather avoid doing this.

@kdy1
Copy link

kdy1 commented Aug 11, 2020

@connor4312 If you need any help, feel free to ping me. I expect the binary size will be 1MB ~ 2MB. Is this acceptable?

@connor4312
Copy link
Member Author

connor4312 commented Aug 28, 2020

Thanks for the offer :)

Running tests again today with the latest of package versions, here's where things are, best time of 3 runs deminifying Gitlens and their compiled bundle size:

Name Time Minzipped Size
estree+escodegen 2096ms 64KB
existing 1711ms 27KB
babel 1186ms 124KB
acorn + astring 638ms 33KB
swc in wasm 578ms 411KB

swc is the winner when it comes to time, but the import cost (for a relatively small feature) is very high. Acorn and astring together are the smallest of the bunch, and also nearly as fast as swc. The formatting is not quite as nice as swc's to my eye (it has a propensity for keeping things on one line, which is not as good for debugging purposes) but it is acceptable. I will explore that direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Code quality issues
Projects
None yet
Development

No branches or pull requests

2 participants