rsm-hcd / AndcultureCode.JavaScript.React.Components

Common atomically designed react components used at andculture
6 stars 8 forks source link

Standardize & Simplify Component Interface Format #104

Open alexgetty opened 3 years ago

alexgetty commented 3 years ago

Some component interfaces have robust inline documentation (like atoms/images/image.tsx) while others do not (like atoms/anchors/anchor.tsx). I feel like typescript interfaces are pretty self-documenting as far as types go, and the documentation in components like Image are redundant and overly verbose by adding 6 new comment lines for every 1 prop line, especially for interfaces with more than a few props. Unless there is an argument for keeping them, I propose we remove this redundant inline documentation. If we do keep it, it should be added to all component interfaces.

Screen Shot 2021-01-28 at 10 37 19 AM Screen Shot 2021-01-28 at 10 37 28 AM

brandongregoryscott commented 3 years ago

Going to have to disagree here - there are certainly cases where behavior cannot be inferred just by prop name, and extra commentary via a JSDoc can really shed some light. Especially if this component library has external adopters - we are more familiar with the codebase and prop naming conventions, but others will not be.

Sure, some of the lines in those JSDoc comments are redundant (such as types, or the @memberof line), those can probably be stripped out as we come across them. A lot of them are probably generated from IDE extensions.

For the time being, I'd advise against a hard rule for adding documentation comments to every single prop or removing documentation comments from every single prop. They're inconsistent because they were pulled from a client project, and there was no tool enforcing a pattern one way or the other. We can definitely look into tooling to add build warnings for missing documentation fields, but based on similar tooling for our C# packages, it's going to take a while to backfill them, and I'm not sure what value it provides just yet.

alexgetty commented 3 years ago

Is there a way to include a description of the prop for JSDoc without adding the redundant @type and @memberof lines for each one? I could get behind that for sure.

brandongregoryscott commented 3 years ago

Is there a way to include a description of the prop for JSDoc without adding the redundant @type and @memberof lines for each one? I could get behind that for sure.

Absolutely. I'm sure there are some in there that don't have them, but a lot of them do, and it's probably from me, admittedly. (The Document This extension for VS Code is what I use) By default it adds the @type and @memberof lines, but I have since updated my settings to exclude those. The generated typescript definition files should take care of the type/member inference for consumers, so there's no real benefit there.

alexgetty commented 3 years ago

Excellent. I'll check out Document This and perhaps once I can resolve some of the backlog of PRs already in the works, I'll go through and clean up the existing interfaces so we start with a solid foundation for future components.

brandongregoryscott commented 3 years ago

@alexgetty I had recently done a pass on a client project that also had them all over the place - the bulk of them can be removed with a quick regular expression.

alexgetty commented 3 years ago

Regular expressions you say? backs away slowly

If you have a chance at some point and could share your process for that, I'd be happy to take a stab at it for this repo.