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

replace prettier with dprint for "deno fmt" #3818

Closed
ry opened this issue Jan 29, 2020 · 12 comments · Fixed by #3820
Closed

replace prettier with dprint for "deno fmt" #3818

ry opened this issue Jan 29, 2020 · 12 comments · Fixed by #3820
Assignees

Comments

@ry
Copy link
Member

ry commented Jan 29, 2020

https://dprint.dev/
https://github.com/dsherret/dprint/tree/master/packages/rust-dprint-plugin-typescript
dprint is a TS formatter written in Rust by David Sherret @dsherret

@nayeemrmn
Copy link
Collaborator

Based on this, #3427 (comment) and #3806, is the goal to not have any std dependent subcommands at all? That would solve a lot of problems. No one's mentioned moving over the test runner which seems like one of the easier ones to move.

@bartlomieju
Copy link
Member

Based on this, #3427 (comment) and #3806, is the goal to not have any std dependent subcommands at all? That would solve a lot of problems. No one's mentioned moving over the test runner which seems like one of the easier ones to move.

That's correct. Test runner is more tricky though. First it shares some functionality with formatter (walking dir tree and finding matching files), so it should wait until we land it. Second, test runner still needs to run tests - this is done by runTests method from std/testing/mod.ts. It definitely needs more thought.

@nayeemrmn
Copy link
Collaborator

test runner still needs to run tests - this is done by runTests method from std/testing/mod.ts

Moving test() and runTests() to ops shouldn't be very difficult -- testing/mod.ts is kind of messy (e.g. it shouldn't have any original exports) so it's hard to tell exactly, but they're quite isolated. The only non-trivial dependency is the glob expander which is easy to replace in rustland and an inevitability in the CLI. No reason to be blocked by the formatter.

Rather, a blocking point of thought is how to enhance the API in light of first-class support. For example, I don't think we need to expose a Deno.runTests() op.

@bartlomieju
Copy link
Member

The only non-trivial dependency is the glob expander which is easy to replace in rustland and an inevitability in the CLI. No reason to be blocked by the formatter.

That's correct;

First it shares some functionality with formatter (walking dir tree and finding matching files), so it should wait until we land it.

I will open up formatter PR shortly, @nayeemrmn I would very much welcome help on glob expander and include/exclude filter.

@kitsonk
Copy link
Contributor

kitsonk commented Jan 30, 2020

Also, I guess this is a breaking change that removes #3033, where some people will have expectations around those configs.

@nayeemrmn
Copy link
Collaborator

Are we going to change the default rules to be more like prettier's? The current ones are yucky.

@dsherret
Copy link
Member

@nayeemrmn see #3824. Let me know about any other changes that should be made or configuration added.

@dev-nicolaos
Copy link
Contributor

Just out of curiosity, what was the motivation for this? Is it that dprint is written in rust and that improves performance, or makes maintenance easier?

Also, the dprint website says it currently only supports ts and jsonc. Does this mean no support for .js files? That seems like it'd be a huge step backwards for deno fmt.

@dsherret
Copy link
Member

dsherret commented Feb 3, 2020

@dev-nicolaos the one used in deno supports .js, .jsx, .ts, and .tsx files and it's much faster than prettier. I would say this is a temporary step backwards, but once we sort out the bugs and implement a few additional features then we will be many steps ahead.

Few points:

@kitsonk
Copy link
Contributor

kitsonk commented Feb 3, 2020

Also, in theory, dprint uses a crate which generates AST for TypeScript. While it is a long long road, we might parse TS/JS in Rust and hand the AST over to the TypeScript compiler for type checking and emitting.

@bartlomieju
Copy link
Member

Also, in theory, dprint uses a crate which generates AST for TypeScript. While it is a long long road, we might parse TS/JS in Rust and hand the AST over to the TypeScript compiler for type checking and emitting.

@kitsonk given we have parsed nad JSON serialized AST, in the present, is there actually a way to load it into compiler just for type check and emit? What changes would our compiler host require?

@kitsonk
Copy link
Contributor

kitsonk commented Feb 3, 2020

is there actually a way to load it into compiler just for type check and emit?

No. See microsoft/TypeScript#33502. I am not sure how much the core team are going to be interested in it, so it might be something that we need to contribute. I also suspect there is a gap between what swc currently produces and what we would need to hydrate an AST for the compiler. It is a long road, but we can chip away at it.

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.

6 participants