denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
96.55k stars 5.33k forks source link

`deno bundle` command will always format output by default #6062

Closed wongjiahau closed 4 years ago

wongjiahau commented 4 years ago

The command deno bundle always format the output by default (even though it's unnecessary in some scenarios), which increase the overall bundling time.

To improve this situation, we should add a --no-format option for the sub command bundle.

ry commented 4 years ago

Can you provide a benchmark of how much time it adds?

I don't like adding extra options. Either we remove formatting entirely - if you can show that it's significantly slower - or it always formats.

wongjiahau commented 4 years ago

@ry I will provide a benchmark, but the difference will definitely be significant when the output get very larges, imagine bundling the index.ts of a React App.

Also, I'll be more than happy if the formatting is removed by default.

ry commented 4 years ago

@wongjiahau A concrete benchmark would really be illuminating for me - thanks. I will definitely remove formatting then.

bartlomieju commented 4 years ago

I'm pretty confident that formatting doesn't have big performance impact on deno bundle. Most of the time is spent on collecting modules and in TS compiler. It will be interesting to see the benchmark.

wongjiahau commented 4 years ago

@ry @bartlomieju I found an example, try downloading this file Terser.js minified bundle.

curl https://gist.githubusercontent.com/wongjiahau/20366c87f922eee462a8ecd172d155cb/raw/ae63b54e386759880590771a3806f1c83231d1a2/terser.min.js > terser.js

Then run:

deno fmt terser.js

It took about 4.31 seconds on my machine. (I timed this manually with my phone)

Output of deno --version:

deno 1.0.3
v8 8.4.300
typescript 3.9.2

My machine specification:

MacBook Pro (15-inch, 2018)
Processor: 2.2 GHz 6-Core Intel Core i7
Ram: 16 GB 2400 MHz DDR4
dsherret commented 4 years ago

I was about to say I wouldn't be surprised if it formatted slow on a minified or extremely large file. I haven't optimized it for minified files, but I bet it's spending a decent amount of time trying to figure out where newlines are (that's something I'd like to improve in the future). Perhaps it could do a quick heuristic to check if the file is minified before formatting.

wongjiahau commented 4 years ago

@dsherret Anyway, why do we need to add this unnecessary overhead when the output produced immediately by Typescript compiler is already formatted when the only difference with the Deno formatter is tab spacing?

wongjiahau commented 4 years ago

Also I would like to add on that, bundled files are usually meant to be consumed by machine rather than human, thus formatting them is like adding ice on top of ice-cream.

kitsonk commented 4 years ago

I tend to agree... I am surprised we are formatting bundles by default. Feels totally unnecessary.

sholladay commented 4 years ago

Shouldn't deno bundle | deno fmt work, for people who need it?

wongjiahau commented 4 years ago

@sholladay True, this is actually resolved in https://github.com/denoland/deno/pull/6085