jeffijoe / typesync

Install missing TypeScript typings for dependencies in your package.json.
MIT License
1.53k stars 22 forks source link

unnecessarily reformats the entire package.json file #102

Closed vassudanagunta closed 9 months ago

vassudanagunta commented 11 months ago

Running typesync reformats the entire package.json file. This forces one to do one of the following:

None of the above are ideal.

In fact, typesync will reformat package.json even if it finds no @types to insert or update!

typesync should produce the absolutely minimal diff to package.json that it can.


P.S. Thanks for the great tool. For me, typesync --dry is better than nothing!

jeffijoe commented 11 months ago

TypeSync serializes the entire package.json using JSON.stringify, and also uses detect-indent to attempt to serialize it back using the same indentation. What formatting is it breaking for you?

vassudanagunta commented 11 months ago

It's not breaking any formatting, but it is changing it. I use many tools that will modify some section of code without reformatting the rest of the file. For example my IDE when I use its refactoring features. Or ESLint when it autofixes errors. In this playground example, notice how ESLint:

I totally understand why you chose the implementation you describe. It is the simplest safe solution, guaranteed to leave a valid package.json. But it is essentially imposing a standard formatting.

If I submit a PR with a solution that is equally safe, would you consider it? Off the cuff, there are a couple of ways to do it:

  1. simplest solution: figure out which typings need to be inserted or updated. Then use very conservative regex to identify the span of source text to replace. e.g. if updating to the latest version of typings for mocha, i'd use regex to isolate the version string following "@types/mocha", and replace it. If there was more than one match in the file for "@types/mocha", I'd bail.

  2. better solution: use a JSON parser that produces an AST with source position info for each node. Use the position info for targeted text replacement.

If I have time, I might explore ESLint's source to see how they do it.

jeffijoe commented 11 months ago

ESLint already parses the source so they have the position and the trivia of each node to be able to re-print the parsed source as-is.

How would you know where to insert new typings? At the end of the dependencies array? That would break the user's expectation of alphabetically sorting the dependencies.

If we do this, it should be behind a flag, like --no-reformat or something.

jeffijoe commented 9 months ago

Closing due to inactivity.