isaacs/tshy

Proposal: post-action / skip idempotent package.json writes

Closed this issue · 10 comments

Running tshy seems to always overwrite the package.json file. This can be problematic because it changes the formatting and causes git changes unless a formatting pass is executed afterwards. I can think of two solutions for this:

  • Check the output that would be written to package.json. If it is structurally the same, don't write it.
  • Add some field to the package.json tshy entry to specify a command to run after tshy, e.g. biome check --fix package.json. Ideally, it can be specified in the same way that it can be specified in the scripts field (that is, with npm binaries being in the path and so on) and ran with the same runtime that's being used to invoke tshy in the first place (node, bun, etc).

Optimistically, this can also work when --watch is enabled.

I'd be down to use polite-json to write the object back with the same formatting, but testing for deep equality feels excessive.

for post commands, I think a better approach is to just use npm scripts for this. Ie, something like:

{
  "scripts": {
    "prepare": "tshy",
    "postprepare": "biome whatever"
  }
}

I'd be down to use polite-json to write the object back with the same formatting

I think this should do the trick. Let me know if you publish a version with this and want me to test it 👍

Seems like it's already been doing that, since 0e73fdf (#46)

Are you seeing that not work somehow?

Last I know this was happening on latest (I checked) less than a week ago. For example, the keywords array would go from single-line formatting to multi-line. I will re-test and double check everything, and then get back to you.

Oh, yeah, I mean, if you have something that's not formatted in a way that JSON.stringify() can do, then it's not going to. If your keywords are all on one line, and you want them to stay that way, and not have the whole thing be an unindented single-line, then don't use tools that edit package.json, because pretty much all of them will blow that away, tshy and npm included.

Hmm, I though the point of polite-json was to respect the pre-existing indentation of the JSON file? Maybe I misunderstood something about it :/

@DaniGuardiola It does, but it doesn't completely re-implement JSON.stringify. It just infers what the indentation and line-break was when it reads the object, stashes it on a symbol prop, and then looks for that when stringifying the object later.

So, eg, if you use \r\n as your line break, and \t as your indentation, then when you go to stringify it, it'll do JSON.stringify(obj, null, '\t').split('\n').join('\r\n') + '\r\n'.

But if you have something like this:

{
  "hello": ["everyone", "in", "the", "world"]
}

then it'll still put those array items on separate lines, because there's no way to tell the built-in JSON.stringify not to.

The alternative is a complicated and error-prone streaming JSON editor approach, which has the added bonus of being anywhere from a thousand to a million times slower.

Also, it just looks for the first indentation/linebreak, so this:

{
 "hello":
  
                          {
"world": [
                      true]}}

will turn into

{
 "hello": {
  "world": [
   true
  ]
 }
}

Okay I understand the issue now and I see why that's not a good approach. Thinking out loud, I think my suggestion here could be a good way to solve it:

Check the output that would be written to package.json. If it is structurally the same, don't write it.

But I also agree with this:

testing for deep equality feels excessive

So, thinking a bit out of the box, what comes to mind is the following idea:

tshy could keep track of any modifications made to the package.json file (a simple let wasModified = false that is set to true in any places where it is altered), and then skip writing the file if none have been performed.

This might not be possible due to the internal details which I'm not familiar with, but just putting it on the table for your consideration :)

That flag would just always be set, though. It doesn't check for the existing exports object before blowing it away with the one it creates from your settings. The only way to do what you're asking would be testing it for deep equality every time. Sorry, not gonna happen. Create your own formatting script if you want, and set it to run after tshy as a postprepare or something.