Improving the structure of the frontend codebase

[essential-randomness]

Premise

The last "annoyances be gone sprint" has highlighted the less-than-ideal shape of our frontend codebase. While the structure of the backend codebase is generally appreciated, and the structure of the components codebase has been polished as part of the sprint itself, the structure of the frontend codebase is still full of puzzling choices, the logic of which mostly resides in my own head.

Goal

Improve the structure of the frontend codebase, making it easier for all contributors (especially beginners) to quickly find their footings.

This discussion starts with an overview of the current codebase structure (this post), and will continue with a proposed solution (next post). Comment as you see fit.

The current top-level folders

I've excluded the standard ones like .circleci/.github.

Shallow folders

This is an overview of the folders with a mostly-flat structure, or that otherwise don't require a lot of explanation.

cache/

This folder contains methods related to updating the values held in the "queryClient" cache. This is extremely tied to our usage of ReactQuery and would benefit from being colocated with the general handling of queries.

pages/

This is a NextJS-specific folder, and it will stay as it is. Files there map to the structure of the site itself.

public/

Also a Next-JS specific folder. These files are accessible as http://[website-url]/file-path-without-public/.

Note that a lot (most?) of these files are not currently used by the software. We should do an effort to get rid of the useless ones at some point.

types/

The type definitions for the frontend, mostly shoved into a types.tsx file. The folder will likely stay, but we should consider splitting up the definition in more files. There's also type definitions generally scattered around the frontend, so we might need to discuss what we want to colocate and what we want to keep here.

I have a general gut feeling we might eventually decide to get rid of this once the server publishes its own types and we can choose whether to derive the frontend ones from them. For now, however, I would consider it as here to stay.

queries/

The top-level queries/ folder (as opposed to the utils/queries/ folder) contains hooks that abstract data fetching with react-query. This is AFAIU considered a best practice (see e.g. https://tkdodo.eu/blog/practical-react-query#create-custom-hooks, from one of the core react-query maintainers).

utils/

Some people say that having a folder named "utils" is a code smell. This folder does nothing to convince me they're wrong (though they might be).

We might want to go through this file by file and see if there's a better place for them once our frontend structure is better defined, and consider putting something here only as a last resource.

utils/queries/

All the functions that do direct calls to the server and return data to the client, or at least all the ones I did before I started creating react-query hooks in queries/ (see above). We should just fold these in the react-query hooks.

Rabbit holes

This is an overview of the folders with a lost of nested folders within. These will likely require the most thought/refactoring.

contexts/

This folder contains only some of the Contexts we have around the codebase. I had moved these in their own folder because I felt that the concept of "context" doesn't really fit in the components/ folder.

The contexts defined outside of the contexts/ folder are:

• components/Auth.tsx: this doesn't even have context in the name, and it's indeed one of the first ones I've written. Should be killed with fire and rewritten. As for its placement, this is a global context for the app that wraps every page (and is indeed imported in _app.tsx).
• components/editors/EditorsContext: the whole "editors" folder is not exactly my proudest code (also a very old part of the codebase). This could also use a refactoring. This is not a global context, and it's specifically added to pages that can actually launch an editor. This might change once we allow opening an editor without a board selected, which makes it possible to create threads from any page.
• components/thread/ThreadContext.tsx: this is used to provide components used in the thread/... pages with computed information about the currently-displayed thread. I have mixed feelings about the necessity of this Context, but I do think it simplifies logic overall. It would do it more, however, is it was better documented AND more thought was given to the information it surfaces.

components/

There are multiple categories of subfolders here:

• "everything related to the page of the same name": boards/, feed/, thread/, setting/, realm/ (though I'd consider this latter as the "realm admin" branch of setting/)
• "components for a very specific type of functionality": editors/, options/ (arguably layout/)
• "anything that starts with use, all thrown together in the same pot": hooks/

An interesting case is the ThreadPreview component, which is used in both boards/ and feed/, and thus stays in the perennial limbo of components/ itself. The top-level files here also include general components that have no specific place (LoadingSpinner, UpdateNotice, LoginModal), and could have maybe gone in layout/. OpenGraphMeta is a global component that handles the open graph definition for each page, and QueryParamNextProvider is not a Context per se, but acts like one and is used globally in _app.tsx.

e2e/cypress/

Most of the folders here are cypress config folders, and are empty, even! The actual tests are in the integration/home folder, and we really don't have as many as we should. I'm strongly considering swapping cypress for playwright, and this folder should go away with the change.

The main sin of this folder is that we have both e2e and tests as top-level folders, which is just confusing.

tests/

This folder is all over the place. There are multiple categories of subfolders here:

• Actual unit test folders: board-utils/, cache/, location-utils, thread-utils
• data/: this contain some standard data definition used across tests. I'm unsure how this is meaningfully different from server-mocks/data.
• server-mocks/: definitions for Mock Service Worker handlers, used to mock server requests. Only used by tests in the UI/ subfolder.
• UI/: component tests, including a special hooks/ folder for testing hooks, and a utils/ folder with (1) jest custom matcher, some code to mock all the required contexts within pages, and some MSW utilities.

We also have a top-level utils.ts file that contains questionable functions to generate a fake post/comment.

Regardless of what we decide to do with tests (e.g. do we want to colocate them as much as possible with the files they're testing? what about e2e tests?), this is not an acceptable structure.

The current top level files

The ones I'd like to get rid off are:

• .babelrc: there has to be a better way, assuming this is even useful at all
• mgdns_proxies.js: sunsetted in favor of bonjour_proxies.js. Everything related should be eliminated.

---

This is all for the "describing current crimes" section. I'm going to absorb this and think about possible solutions, then I'll write an actual proposal in the next few days. Comments are open and encouraged for any questions/thoughts.

[essential-randomness]

After reviewing various opinionated guides on how to structure NextJS projects, here's the newly proposed structure.

A global change

To make it easier to distinguish project and configuration files, we will move most folders within a src/ directory, which NextJS supports by default.

Note: we'll add special configuration so that import paths can avoid using the src/ prefix. So import X from "components/core/editors" will import from src/components/core/editors.

Simply move under src/

• pages/: a special NextJS folder.
• public/: a special NextJS folder.
• types/: we can discuss how to split these later.

src/components/ folder

components/ (now src/components), will use the following structure:

• components/core: components that are used all over the place. Example: the current components/editors folder and components/layout folder.
• components/pages: follows the structure of the pages/ directory. Includes the special _app/ folder for top level components that are used exclusively in _app.tsx.

Note: we can choose to forgo the pages/ folder if we desire, and directly mimic this structure in components/. This is a common choice, and it allows us to maintain a flatter structure. I suspect it might become confusing later on, but maybe we shouldn't prematurely optimize.

Newly proposed

• components/core/posts: anything related to the display of the "post-looking things" themselves. Will include:

  • components/ThreadPreview.tsx
  • components/thread/ThreadPost.tsx
  • components/options/usePostOptions.ts
  • components/options/useTagsOptions.ts

• components/core/comments: anything related to the display of the "comment-looking things" themselves. Will include:

  • ThreadComment, from components/thread/CommentsThread.tsx.
  • components/options/useCommentOptions.ts

Note: there are methods that apply to both threads, posts and comments (e.g. "scroll to thread entity"), but we don't currently have a name to group all 3 categories in one—except thread entity, obviously, which I'm not in love with. If we find a good term, we could put all these under the same core/[term] folder.

Changes in src/pages/

These will have influence on the actual URLs of our pages. I don't foresee these as an issue, except for the thread one. We can use a redirect (for now), and maybe get rid of it in the future if it starts causing issues—we're still in alpha, so we're allowed to break things a bit, as a treat.

• Move pages/realms/admin to pages/settings/realms/current
• Move pages/users/settings to pages/settings/personal
• Move pages/users/feed.tsx to pages/feeds/personal.tsx
• Controversial choice alert: move pages/[boardId]/thread/ to pages/[boardId]/threads (with thread plural), and consider always using plurals for entities in paths (similar to REST apis). See above notes for redirects.

A weird case

A board is a special kind of feed, and as such they might need to share elements even if the pages exist at the same level. Shared elements should be placed in components/pages/feeds, as the most generic option.

src/lib/ folder

src/lib/ folders will include all shared code that is not specifically a component, a hook, or a Context. In particular we will:

• Move all the utils/NAME-utils.ts files to src/lib/NAME.ts files UNLESS we can find them a better place within components/.
• src/lib/api will include all the handlers for client<=>server communication.
  • src/lib/api/hooks: contains custom hooks for react-query within the hooks subfolder, functionally hiding our usage of react-query from the rest of the codebase.
  • src/lib/api/cache: contains utilities used to update the client cache on server mutations. Ideally, this will be used by hooks directly, and not be imported outside of lib/api.

Final structure

This is an example of the eventual final structure, with examples of files that we'd move in each place.

- src/
  - lib/
    - api/
      - cache/
        - ...
      - hooks/
        - ...
    - scroll.ts
    - location.ts
    - router.ts
    - ...
  - components/
    - core/
      - comments/
        - ...
      - editors/
       - ...
      - layouts/
        - Layout.tsx
        - LoadingSpinner.tsx
        - LoginModal.tsx
        - useLoggedInOptions.tsx
      - posts/
        - ...
      - useFromBackButton.tsx
      - useStateWithCallback.tsx
      - useIsChangingRoute.tsx
      - ...
    - pages/
      - _app/
        - AuthContext.tsx
        - OpenGraphMeta.tsx
        - QueryParamNext.provider.tsx
        - UpdateNotice.tsx
        - ...
      - boards/
        - threads/
          - ThreadViewContext.tsx
          - useStemOptions.tsx
          - ...
        - BoardSidebar.tsx
        - BoardBottomBar.tsx
        - ... 
      - feeds/
        - FilterableContext.tsx
        - FeedBottomBar.tsx
        - ...
      - invites/
        - ...
      - settings/
        - realm/
          - CreateInvitesPanel.tsx 
          - ...
        - user/
          - UserSettings.tsx
          - DecorationSettings.tsx
          - ...
      - RealmContext.tsx
      - ...

Tests structure remains TODO in a latter update.

[virtuous-cat]

I'm good with this overall structure.

I don't love having, for instance, ThreadPost in core/posts. I would prefer it go in pages/boards/threads. I feel like core/posts should be for things that are shared between posts in multiple places, and things that are only for posts on a specific page should go in that page's folder

And then we can solve the things-common-between-feeds-and-boards problem by putting them in a core/feeds folder.

Move pages/realms/admin to pages/settings/realms/current

When I was coding this, I wanted to make sure it was clear when you are dealing with realm administration vs a user's settings for the realm. I'm not opposed to changing this url, I get why you want to have it under settings, but I'm a little concerned that if you are just looking at the url .../setting/realms/current by itself you can't tell which it is. If we do this, I'd want to make sure that the link text and page title keep "administration".

I'm pro plurals in paths

[essential-randomness]

Thinking about it, you're right about not creating the core/posts folder right now. The reason I proposed it is I believe we'll need to extract it at some point, but I do believe it's premature to have it, and we should generally avoid doing things prematurely. Let's do core/feeds instead!

I see and agree with your point about administration vs personal settings. It might make sense for the two things to be completely separate. I was trying to think about how to frame this in our "pro-plurals" mode, but I'm struggling to find something I like. I'm kinda tempted to divide it between settings for personal settings, and configurations for administration stuff, but IDK if it's clear. Would love some brainstorming on this, either here or on Discord.

[virtuous-cat]

I didn't actually mean don't have a core/posts folder at all, my point was more about what conceptually should go in core/[thing] folders vs pages/[thing or parent of thing] folders.

I don't know that we have to be so strict about our plurals that we can't use words that don't have natural plurals. Even github uses plural where it makes sense to, but also github.com/marketplace. So we could do .../settings/admin/realms/current or settings/realms/admin or something.

[essential-randomness]

Oh, woops! You're right, I jumped ahead with the core/posts thing. I'm actually unsure that the other components there are used in more than one place if you remove ThreadPost, so that's why I was talking about removing them and putting those directly into pages/boards/threads too. But let's see whether that's true in practice and make the necessary adjustments.

Good point about not being strict with plural. I think the big underlaying question here is how we want to groups these settings:

• if we do settings/realms/admin I expect that to be in contrast to e.g.settings/realms/user. That makes me think we're dividing settings by category, and then putting both admin and user settings in the same category.
• if we dosettings/admin/realms, then I expect us to heavily distinguish between user and administrative settings. I think it's reasonable to foresee us having administrative settings that are not realm related (sosettings/admin/[other category]), but IDK what they could be off the top of my head.

[virtuous-cat]

Oh, yeah, ThreadPreview should end up going into core/feeds since it's used in both feeds and boards (and possibly get renamed? It always takes me a second to remember what it is). And then I think the options hooks are used in both ThreadPreview and ThreadPost so my thought would be to keep them in core/Posts, but I can see the argument for putting them into core/feeds for now.

And I don't actually see a scenario for comments outside of threads, so I don't think a core/Comments folder is necessary.

I think it makes the most sense to have the base grouping for the settings be settings/admin and settings/user (we do have to decide user vs personal), particularly since I think those should definitely remain separate pages and they are both set up to have multiple panels within those pages.

[virtuous-cat]

Summary of changes to the initial proposal discussed so far:

(Ms. Boba, I am assuming you agree with my suggestions from my last comment above, squeak if not :P)

src/pages folder:

• Move pages/realms/admin to pages/settings/admin/realms/current
• Move pages/users/settings to pages/settings/personal
  (components/pages folders will also need to mirror these changes)

src/components/core folder:

• Create core/feeds folder and put anything shared by boards and feeds here, i.e. ThreadPreview.tsx, but also FilterableContext.tsx for example, as well as anything used by posts in both boards/feeds and threads (for now this is only usePostOptions, if we end up with more of them in the future, could make core/posts).
• Don't create core/comments or core/posts for now.
• As a general principle, anything that is only being used in a specific page should go in that folder in src/components/pages.

Additional thing for discussion:

In terms of where things currently in the utils folder are going, here are my thoughts:
boards-utils.ts --> scr/components/pages/boards/boards-utils.ts (I don't want to change the file name to just utils because it makes searching hard)
threads-utils.ts --> scr/components/pages/boards/threads/threads-utils.ts
typescript-utils.ts --> types/typescript-utils.ts
embeds-cache.ts --> ??? this doesn't actually seem to be being used anywhere? Is it doing something?
utils/queries folder --> these are our axios requests, they are then being used in our api hooks, either in the queries folder or currently directly in components where we have yet to pull them out into seperate files. Do we want to move the folder to src/lib/api/requests (don't know if that's the best folder name) or do we just want to fold these methods into the hooks files?
Everything else: utils/NAME-utils.ts --> src/lib/NAME.ts