rushstack icon indicating copy to clipboard operation
rushstack copied to clipboard

[rush] Rush example *.json files are hard to read and setup

Open CobyPear opened this issue 2 years ago • 14 comments

Rush example *.json files are hard to read and setup

Summary

I am setting up a monorepo with rush, (using rush init) following the official docs. I have added a package to my repo and I am going through the rush.json and version-policies.json. These files contain some mistakes including duplicated properties which cause errors when trying to use rush update.

For example, https://rushjs.io/pages/configs/version-policies_json/ has 2 objects. At first they seem to be different but after looking closer both start with the same (Required) property. But if I run rush update with both of these objects, the command fails. If I run it again with just 1 object in the array, the command runs successfully.

These .json files are very hard to read and navigate. I'm not one to disparage code comments but this is a bit much! For example, 'Required' properties can be buried. I think the link to the schema solves a lot of this. I like the idea of having this documented file in the docs, but it's a lot to look at when starting a new monorepo.

I see the option to skip the .json files and use the rush-lib API, however the docs don't really say (or I couldn't find) where that file should go or what it should be named or anything!

Repro steps

Expected result: When I initiate a new monorepo with rush init, the .json files contain sane defaults and are easy to read and customize

Actual result: .json files are full of comments making them difficult to navigate and actually use. Docs are somewhat unclear about using the rush-lib api instead of rush.json.

Details

Happy to help clean these things up when I have time.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version? 5.62.4
rushVersion from rush.json? 5.62.4
useWorkspaces from rush.json? na
Operating system? Mac
Would you consider contributing a PR? Yes
Node.js version (node -v)? 14.18.3

CobyPear avatar Feb 17 '22 15:02 CobyPear

Agree, I think the "$schema" entries solve this as mentioned, as well as the docs online. I find my self spending a bunch of time each new repo setup manually deleting all the comments in the json files.

prescience-data avatar Mar 05 '22 09:03 prescience-data

I've gotten similar feedback on this, people disliking all the comments.

I think the json comments were a good simple start for rush, but now maybe it's time to move these things to the documentation if not already there?

wbern avatar Mar 10 '22 05:03 wbern

What IDE are you all using that supports the comments in the json file? This should be a yaml file if you want the comments, or perhaps a json5 file. Here is what I see in the latest IntelliJ.

image

oravecz avatar Mar 15 '22 00:03 oravecz

@oravecz Using vscode I was able to configure 'comments in json'

CobyPear avatar Mar 15 '22 00:03 CobyPear

First I apologise if this not the right place for this but it seem relevant to me( i getting a little lost with how everything fit together).
Can someone explain why API-extractor and other rush tools is even using the .json extension why not just change it to .json5 see(https://github.com/json5/json5) or use some other file type that support commit in json. IMHO it would solve this type of problem and make setup a lot simpler.

frog-o avatar Aug 28 '22 10:08 frog-o

First I apologise if this not the right place for this but it seem relevant to me( i getting a little lost with how everything fit together). Can someone explain why API-extractor and other rush tools is even using the .json extension why not just change it to .json5 see(https://github.com/json5/json5) or use some other file type that support commit in json. IMHO it would solve this type of problem and make setup a lot simpler.

We tried to switch to JSON5 along time ago in https://github.com/microsoft/rushstack/issues/1088 but were unable to establish consensus (see that thread for details).

It's been proposed again recently in https://github.com/microsoft/rushstack/issues/3615 -- let's use that issue to discuss JSON5.

octogonz avatar Sep 13 '22 00:09 octogonz

For example, https://rushjs.io/pages/configs/version-policies_json/ has 2 objects. At first they seem to be different but after looking closer both start with the same (Required) property. But if I run rush update with both of these objects, the command fails. If I run it again with just 1 object in the array, the command runs successfully.

Let's pick a different example to discuss besides version-policies.json. 😅 That config file is ancient and did not follow best practices. An array should never be the top-level data structure in a config file.

Maybe api-extractor.json or command-line.json would be better conversation pieces.

I've gotten similar feedback on this, people disliking all the comments.

I think the json comments were a good simple start for rush, but now maybe it's time to move these things to the documentation if not already there?

Rush's config file design is inspired by Linux /etc files, where a config file is basically a big input form showing every possible setting that you can fill out. When you upgrade the tool, the config file and its comments typically get upgraded as well. The premise is that you have tons of config files under /etc from different tools, and it's not always convenient to figure out what tool the file is for, then find the website of that tool, then go looking for the right version of the docs corresponding to whatever release is installed. Instead, when you open the config file, all the docs for each setting are right next to the setting. And every possible setting is clearly enumerated.

The discussion above pointed out some downsides of this design. It also overlaps with JSON Schema, but (sadly) schemas were primarily designed for finding mistakes. They can document individual fields, but they have no way to present full examples. For polymorphic lists (e.g. "animals": [{"kind": "cat", "color": "gray"}, {"kind": "bird", "wings": 4}] there's no way to access the help for "wings" without guessing that "kind" is a discriminator and then creating an entry with "bird". Because of this implicit structure, trying to read .schema.json files is not a great experience either.

To move this discussion along, could someone point out some other software that has a config file experience that you really like? It needs to face similar requirements as Rush:

  • Lots of different config files with lots of settings
  • Complex nesting of settings
  • A stampede of users working in a huge monorepo, who need to work with these config files. Many of these people may be unfamiliar with Rush or Heft, and probably can't easily guess which file goes with which website.

octogonz avatar Sep 13 '22 01:09 octogonz

I find my self spending a bunch of time each new repo setup manually deleting all the comments in the json files.

@prescience-data Why is this difficult? You don't just delete everything below "$schema"?

octogonz avatar Sep 13 '22 01:09 octogonz

I find my self spending a bunch of time each new repo setup manually deleting all the comments in the json files.

@prescience-data Why is this difficult? You don't just delete everything below "$schema"?

I use the existing properties as a guide as I've found they change (infrequently) between versions (as well as new files emerging), making it not ideal to copy and paste the content from the same file in another project.

Clearing the entire json to an empty one and then manually typing in all the properties again with intellisense to me qualifies as "difficult" (when compared to having a clean json file from the start where this toil is not required each new project).

My preference would be rush init cli flag (ie --noJsonComments) that perhaps toggles the comments on or off during setup - I would be happy to work on a PR for this if it's something likely to be merged?

prescience-data avatar Sep 13 '22 01:09 prescience-data

🤔 Instead of rush init --no-comments, what if we provided a CLI tool that can remove comments and re-add them? With our current model, there is already a problem of "upgrading the comments" when you upgrade the tool.

Consider shell commands like this:

# Deletes all the "/** */" doc comments from rush.json.
# Maybe it preserves "//" comments which we reserve for user comments
undoc-config rush.json

# Restores all the "/** */" comments in rush.json:
doc-config rush.json

Maybe the second command works generically by fetching the "$schema" URL to obtain the latest published content for the config file. Thus it could work with any config file, for example Heft or API Extractor. I mentioned earlier that the JSON Schema file format is not powerful enough to express a full template (e.g. samples of polymorphic list items), but that's not difficult to solve (maybe example.schema.json has an accompanying example.template.json).

octogonz avatar Sep 13 '22 07:09 octogonz

A different idea would be a VS Code extension that displays the comments inline. The JSON Schema support is sort of like this, but the tool tips aren't as expressive as comments:

JSON Schema experience: image

How the template looks: image

However, maybe we could build a VS Code extension that displays richer documentation inline, without having to modify the file contents.

Although, it can be helpful to have the doc comments in contexts outside VS Code, for example when browsing a GitHub repo:

https://github.com/microsoft/rushstack/blob/b13bd76f5daaa47e19692edc6149451576052107/common/config/rush/experiments.json#L1-L15

octogonz avatar Sep 13 '22 07:09 octogonz

If people have other ideas, please share them. In particular, if you can point to some other software whose config files are really awesome, that would be very helpful.

(BTW I'm not disagreeing. I hear what you are saying above. Just trying to collect more possible design alternatives to inform this discussion.)

octogonz avatar Sep 13 '22 07:09 octogonz

CC @elliot-nelson @D4N14L

octogonz avatar Sep 13 '22 07:09 octogonz

🤔 Instead of rush init --no-comments, what if we provided a CLI tool that can remove comments and re-add them? With our current model, there is already a problem of "upgrading the comments" when you upgrade the tool.

This would be a good (better) solution as well as it: a) is agnostic for any changes over time to json files, and; b) also be flexible enough allow the rush init --no-comments, as the --no-comments flag would simply call the underlying rush undoc command post-init.

prescience-data avatar Sep 13 '22 09:09 prescience-data