devongovett / dprint-node

A node API for the dprint TypeScript and JavaScript code formatter
MIT License
483 stars 11 forks source link

Configurable Plugins #90

Open ayuhito opened 2 years ago

ayuhito commented 2 years ago

This project is great but it seems limited to just JS/TS which I feel could be expanded upon. I personally need to use the JSON plugin which isn't possible with this.

Possibly we could interact with it like so?

dprint.format(filePath, code, language, {
  lineWidth: 100
});

Alternatively, we could split each language by function.

dprint.formatJSON(filePath, code, {
  lineWidth: 100
});
dsherret commented 2 years ago

You might want to check out: https://github.com/dprint/js-formatter#nodejs

stephenh commented 2 years ago

Ah nice, thank you @dsherret ! ... this is maybe a naive question, but @devongovett what's this value prop of this dprint-node package over the @dprint/typescript package? Do they use like different build or deployment approaches?

If there is a material difference, it'd be great to highlight this on the dprint-node readme.

Thanks!

stephenh commented 2 years ago

Oh, I know the difference now! :-)

In looking at flamegraphs of our codegen process, @dprint/typescript is paying a ~500ms-on-my-laptop cost to like reload the WASM in getBuffer on every single invocation of node:

image

(There are three getBuffers, one for @dprint/typescript and two for @dprint/json b/c I have two separate places generating/formatting JSON and currently not sharing the getBuffer instance.)

Contrast this with dprint-node, this part of the flamegraph just goes away.

This also shows up in execution time, i.e. @dprint/typescript takes ~800ms to generate a very trivial output (I'm being lazy and using tsx so disclaimer that transpilation is included in the 800ms):

[I] ~/o/ts-poet (master|●1✚4…) $ time node --loader tsx ./bench.ts
(node:30271) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
if (true) {
  logTrue();
} else {
  logFalse();
}

________________________________________________________
Executed in  819.30 millis    fish           external
   usr time  842.56 millis  558.00 micros  842.00 millis
   sys time  123.40 millis   68.00 micros  123.33 millis

But with dprint-node, the same code runs in ~200ms:

[I] ~/o/ts-poet (master|●1✚4…) $ time node --loader tsx ./bench.ts
(node:30620) ExperimentalWarning: Custom ESM Loaders is an experimental feature. This feature could change at any time
(Use `node --trace-warnings ...` to show where the warning was created)
if (true) {
  logTrue();
} else {
  logFalse();
}

________________________________________________________
Executed in  229.45 millis    fish           external
   usr time  201.29 millis    0.00 micros  201.29 millis
   sys time   83.14 millis  548.00 micros   82.60 millis

With the dprint-node flamegraph showing tsx/esbuild now dominates the runtime.

So for now I'll probably use dprint-node; while I can't claim a half-second is really making-or-breaking our code generators performance, i.e. we spent ~few seconds gathering input data from our database/etc.; but when looking for hot spots, the WASM loading is currently the "next biggest thing" to fix.

@dsherret IANAE on node/WASM, but if there was a way for @dprint/typescript to cache the WASM loading process across invocations of node, that'd be great.

@devongovett if you'd like, I can submit a PR to try and work a tldr of the "wasm vs. binary" difference into the dprint-node readme.

dsherret commented 2 years ago

@stephenh I'd rather get the @dprint/typescript-like npm packages eventually distributing Node API plugins (the buffer solution was a quick first pass). The issue I see with dprint-node is that it doesn't follow the plugin model of dprint, but rather distributes specific plugins at specific versions.

stephenh commented 2 years ago

@dsherret ah gotcha! Cool, that makes sense.

Personally I'm not that bothered by the specific-plugin-at-specific-version approach if it's the most pragmatic way to get native binary versions of dprint into Node, but agree that unpinned/user-provided plugins sounds like an ideal end-state goal.