rsm-hcd / AndcultureCode.JavaScript.React.Components

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

Move component-specific files #91

Open alexgetty opened 3 years ago

alexgetty commented 3 years ago

Similar to moving scss files into component folders, I think we should consider moving component-specific interface/utility files into the component folder. It aligns with our goal of keeping component files together and allowing them to be more self-contained.

Related to #86, but not explicitly documented there, so I wanted to break it out as a separate issue.

brandongregoryscott commented 3 years ago

Hmm, not sure I'm following. Are you suggesting we split out the props interface for each component into a separate file in the same folder? I'm not seeing many interfaces that are defined outside of the component that uses them, except maybe InputProperties that is shared across a few components.

I'm not sure I agree, especially for something like utility files - they are generally built out to make your life easier when interacting with specific objects or types, and not just one specific component. From an organization perspective, I could see the public exports in index.ts becoming a mess if we are exporting from component folders as well as the utilities folder.

Can you provide some examples interfaces/utilities where they currently are and where you're proposing they go?

alexgetty commented 3 years ago

Thanks for the feedback! I definitely agree that general utilities shouldn't follow this pattern, I was talking specifically about external files that are leveraged by specific components and those components only. In my experience, having all files directly related to a component in the same place makes it easier for newcomers to get a mental model of everything involved in a component's functioning, as well as make it easier to maintain if any changes need to occur.

Here's some examples:

src/atoms/interfaces/svg-icon.ts is only imported into Icon component files within src/atoms/icons, so it should live in that folder.

src/atoms/interfaces/input-properties.ts is only imported into the components in the src/atoms/forms folder, so it should live in that folder. (side note, another proposal I submitted (#90) would merge the various input components into a single input component to better reuse shared functionality, so if that is approved this interface could live in the component file itself.)

in src/atoms/constants there are button-sizes.ts, button-styles.ts, and button-types.ts. Since they related directly to the Button component, they should be located in src/atoms/button along with the rest of that component's files. Same would go for the other component-specific files in src/atoms/constants like heading-sizes.ts, icon-sizes.ts, icons.ts, svg-icons.ts etc.

in src/utilities, toast-manager.ts (and it's corresponding test file) should be moved into src/atoms/toasts since it is specifically for the toast component. Likewise, the icon-utils.ts file in that same folder should be moved into the src/atoms/icons folder.

General utilities can remain where they are since they are not component-specific, like src/constants/keyboard-keys.ts, src/enums/accessibility-labels.ts or src/utilities/core-utils.ts to name a few.

brandongregoryscott commented 3 years ago

@wintondeshong @Stefanie899 How do we feel about this proposed restructuring? I think it's a bit different from our current pattern, though as we all know the frontend tends to diverge from naming/structuring patterns eventually anyway. My main concern would be that it leads to more ambiguity when creating some of these constants or utilities - right now, they may only be used in a single related component, but where else might they be imported throughout the project?

wintondeshong commented 3 years ago

I can get behind it if the interfaces and/ or other files are truly only related to that component. The idea being that as soon as any of those files are used elsewhere they get lifted to a different directory. Also if they aren't the literally same concept and are a different level of abstraction, then they also should be moved for reuse.

Stefanie899 commented 3 years ago

Agreed, if they're used elsewhere they should live higher up the chain than directly with the component. Beyond that, I'm on board with them living with the component.

alexgetty commented 3 years ago

I agree @wintondeshong and @Stefanie899, if something is used in multiple places, it wouldn't apply to the proposal I made. This would just be for things specific to a component. If we find that we could be using a helper from one component somewhere else, it should be bumped up to a more generic place.