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

Suggestions for Enhancing Code Organization and Quality #804

Open
alexlwn123 opened this issue Feb 9, 2024 · 5 comments
Open

Suggestions for Enhancing Code Organization and Quality #804

alexlwn123 opened this issue Feb 9, 2024 · 5 comments
Labels
enhancement improvements to existing features

Comments

@alexlwn123
Copy link
Contributor

alexlwn123 commented Feb 9, 2024

Hi everyone!

I'm Alex, I've recently started diving into the Stacker News codebase. As I've been getting more familiar with the repo, I've identified a few opportunities that I believe could help us improve the codebase's organization, maintainability, and overall development experience.

As these ideas carry architectural significance, I'm sharing these now as an RFC to ensure we reach consensus before moving forward with substantial implementation efforts. Also, I'm happy to contribute or lead development on any of these that community deems appropriate.

Code Organization & Quality Ideas

1. Component Directory Organization

The current flat structure of the components directory seems a bit challenging to navigate due to its size. By organizing components into feature-based directories, we could enhance discoverability and maintainability of our components.

As a rule of thumb, components that appear close to each other on screen should have files close to each other in the codebase. (components/item, components/profile, components/common, etc.)

2. Breaking up Component Files

Currently, many component files actually contain 5+ composite components that contribute to a larger component see here. I suggest that as a practice we should shoot for one component per file (with reasonable exceptions). While this may introduce file clutter, by first implementing the proposed feature-based directory structure, we can keep the repository organized and navigable.

3. Type Safety

The project's reliance on vanilla JavaScript brings some challenges in understanding and extending the codebase. To help DX and reduce the overhead of investigating features or bugs, I suggest we incrementally introduce type safety.

While migrating to TypeScript is an option, a more immediate and less disruptive approach could be to incrementally introduce JSDocs. JSDocs inherit most of the features of TypeScript that most devs care about, namely editor type inference and better linting.

This might seem like a monster, but JSDocs can be added incrementally and have zero effect/risk on runtime. Also, we can make significant progress quickly by starting with the GraphQL hooks/fragments and using a tool like GraphQL Code Generator's JSDoc plugin.

4. Test Coverage

While complete coverage is certainly overkill, I think the project would benefit from a modest amount of frontend integration testing being added to the CI pipeline. A ton of tricky bugs can be caught through snapshot testing (via the React Testing Library.

With frontend testing, I think a little goes a long way. Just adding a check that each page mounts to the CI will catch a ton of bugs. In addition, this really helps with confidence that new features won’t break the site, especially as more contributors join.


I'm looking forward to your thoughts, feedback, and any additional ideas you might have! As mentioned, I'm happy to contribute or lead development on any of these ideas community deems appropriate.

Cheers.


edit: see additions

@alexlwn123 alexlwn123 added the enhancement improvements to existing features label Feb 9, 2024
@huumn
Copy link
Member

huumn commented Feb 9, 2024

I think these are all great suggestions! Which of those that you list is most important do you think?

One I'd add is making it trivial to get started contributing to SN like building in a regtest lightning network to the dev setup or some kind of process for delivering a prod subset seed. We were talking about this a little on SN yesterday.


For things that require ongoing an commitment by the team and are at base a matter of taste, I'd prefer these kinds of things are done by someone working full time for SN.

@alexlwn123
Copy link
Contributor Author

Thanks!

Which of those that you list is most important do you think?

I'd rank their importance as 3 > 1, 2 > 4.

1 & 2 probably qualify as a matter of taste, so happy to hold off.

@ekzyis
Copy link
Member

ekzyis commented Feb 9, 2024

I also like these suggestion, thanks for creating this important ticket in such detail and initiating the discussion :)

I especially like the incremental approach: then we can get experience while working on other things and without committing too fast too much to one particular approach. Also, changing a lot of the code base in one branch gets old fast since you're going to have conflicts with basically any other WIP branch.

One thing that makes sense to include when working on 1. and 2. is to add module aliases imo.

@ekzyis
Copy link
Member

ekzyis commented Feb 11, 2024

Started to add some tests in #808. Let me know what you think :)

@mzivil
Copy link
Contributor

mzivil commented Feb 12, 2024

thanks for initiating the discussion! i've had similar thoughts as another newcomer. here are some of my related ideas and thoughts on code organization. i +1 the type safety suggestion as well as the team grows.

  1. Component Directory Organization

+1 on feature-based directories. i would call the top-level folder features instead of components (features/item, features/profile, etc) and then dump all feature related files there. Alternatively, you can further break it up by file type (features/item/components, features/item/hooks, features/item/utils) as the folders grow but that might be overkill for now.

  1. Breaking up Component Files

+1 on leaning towards fewer exported components per file without being too strict on the 1 component per file rule.

An idea that might help with this is moving shared base UI components into a separate directory from features, like /ui or /design, that contains buttons, forms, inputs, text, heading, colors, layout, & other shared design components. Think of it like a light-weight design system package that other projects (and internal/admin apps) could use to adopt SN's styles. Also makes it easier to restyle and add/remove functionality globally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improvements to existing features
Projects
None yet
Development

No branches or pull requests

4 participants