Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move component-specific files #91

Open
alexgetty opened this issue Jan 26, 2021 · 6 comments
Open

Move component-specific files #91

alexgetty opened this issue Jan 26, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@alexgetty
Copy link
Contributor

alexgetty commented Jan 26, 2021

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.

@alexgetty alexgetty changed the title Move component-specific interfaces Move component-specific files Jan 26, 2021
@brandongregoryscott
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

@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?

@brandongregoryscott brandongregoryscott added the pregroom Item still requires definition label Feb 3, 2021
@wintondeshong
Copy link
Contributor

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
Copy link

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.

@brandongregoryscott brandongregoryscott added enhancement New feature or request and removed pregroom Item still requires definition labels Feb 17, 2021
@alexgetty
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants