TBS-EACPD / infobase

Source code for GC InfoBase / Code source de l'InfoBase du GC
https://canada.ca/GCInfoBase
Other
29 stars 5 forks source link

More obvious directory structure #1612

Open AlexCLeduc opened 4 years ago

AlexCLeduc commented 4 years ago

It's kind of difficult to figure out where something goes right now. I'd like to propose we reset our directory structure.

I don't have all the answers (warning: there will be conflicting ideas below). Here's a starting point for discussion:

What about core/ ? explorer-common/ ? search/ ? I have 3 patterns in mind to take care of the rest

1. Using recursive utils/ and components/

To avoid src/utils getting too large, we can try and have multiple utils/ directories. If a utility becomes re-usable beyond its directory, it gets upgraded to a higher-level directory's utilities, until it ends up in src/utils/. For instance, utilities for defining new routes can be in sections/utils, because it won't be used outside sections/. Same thing goes for components.

Since core/ has a lot of components that only get used in App.js, and a lot of bootstrapping stuff only used in src/InfoBase, most of it can be dumped into src/InfoBase/'s components and utils directories.

Another benefit of this pattern is that removing a section or panel will also remove all of its specific utils/components.

2. domain/

one problem with just having plain utils/ and components/ is that we have plenty of tightly related tools that contain both plain functions and components:

For lack of a better idea, we can nest stuff like this under src/domain/. I think we'll end up with more of these as we grow, so I'd prefer not to keep them top-level.

Note: Let's try and find a better name than domain

3. Platform/ (or foundation/)

Certain utils/components are gonna be used broadly and are more context-aware than simple functions. This could be another split point.

Think about

Conclusion ?

We can use any combination of recursion, domain, platform. If we use both domain and platform, we should make sure the difference is clear (e.g. should glossary/footnote utils go in domain, or platform?)

Here's a more detailed breakdown of the all the top level files (and core). Blank destination means leave it alone. If we have more opinions we can move this to a google doc or sheet. There is some stuff missing. For instance, we may decide to move some context-aware or smart components outside of components/ and into platform/ or domain/.

Current file new destination
EstimatesComparison sections/
FAQ sections/
IgocExplorer sections/
InfoBase no change
InfoLab sections/
PrivacyStatement sections/
TagExplorer sections/
TextDiff sections/
TreeMap sections/
about sections/
app_bootstrap InfoBase/
charts
common_css static/
common_text static/
components
contact sections/
core/DevFip.js InfoBase/components/
core/EasyAccess.js InfoBase/components/
core/ErrorBoundary.js InfoBase/components/
core/InsertRuntimeFooterLinks.js InfoBase/components/
core/NavComponents.js (and scss) components ? sections/components/ ?
core/Spinner.js components/
core/Statistics.js unsure, probably utils ?
core/SurveyPopup.js InfoBase/components/
core/TableClass.js tables/
core/analytics.js utils?
core/breakpoint_defs.js constants/
core/color_defs.js constants/
core/color_schemes.js constants/
core/feature_detection.js utils ?
core/format.js unsure should probably be split up. Also has side-effect of declaring handlebars
core/lazy_loader.js utils? models?
core/reactAdapter.js utils?
core/tables move alongside all the other tables
email_backend_utils.js utils/ ?
explorer_common unsure
extended_bootstrap_css static
general_utils.js utils/
glossary sections/
graphql_utils utils/
handlebars Infobase/ ? Later: consider splitting it up, making panels import their own special helpers via a side-effects.js
home sections/
icons static/ ? components/ ? constants?
infographic sections/
link_utils.js linking/ (or platform/)
metadata sections/
models
panels
partition sections/
png static/
request_utils.js utils/
robots static/
rpb sections/
search unsure
svg static/
tables consider moving under models/ ?
ktw1016 commented 3 years ago

I really like the overall idea of all of this. I like the structure you laid out as starting point.

  1. Using recursive utils/ and components/

I like this idea and we should try it out. I'm unsure how easily manageable it will be to keep separate utils and components but I think it's worth a shot

But I'm not understanding domain and platform completely. Are they basically utils and/or components but doesn't quite fit into either, so you throw them in there? Can you clarify your thoughts on the difference between the two and, (why and how) would something fit into one or the other, instead of utils or components?

AlexCLeduc commented 3 years ago

Initially I was thinking domain would be more business-related concepts and platform would be globally setup stuff like analytics, but looking through our components dir right now, I like the kitchen sink approach and don't think business-related stuff needs to be separate.

OTOH, I think there are still groups of components and utilities that we'll want to colocate, and that are often imported together. Also, some components/utils require external setup, like DocumentDescription or DocumentTitle, or modules/singletons that hold state.

If we think those criteria merit separation, we can have something like

foundation | platform | domain/

The criteria for going in this directory can be

  1. modules that colocate utilities and components that are often imported together
  2. hold state in a singleton or in the module (e.g. top level let)
  3. have dependencies on side effects (e.g. assume that the page has a #header selector)
Stephen-ONeil commented 3 years ago

Been trying to develop some thoughts on this/sketch a proposed restructuring. Heavily based on earlier discussion. Not sure I'm settled on this, but I've started to spin my tires so it's probably a good time to put it back up for discussion.

Proposal: a recursive directory structure based on four types of directories 1) utils

Sketch of proposed structure:

AlexCLeduc commented 3 years ago

Seems good to me, just to make sure I understand, the root platform/ would have the explorer and search?

ktw1016 commented 3 years ago

For models/, would populate files go into platform? If it's just ensure_loaded in platform, I think ensure_loaded can just stay on the root of models/

I'd generally prefer to minimize the use of recursive subdirectories of components/, platform/, utils/, etc if it's just 1 file in there just so the whole directory structure tree doesn't become too huge.

I think it's worth keeping charts/ and panels/ at root directory for more visibility

AlexCLeduc commented 3 years ago

Agreed with @ktw1016 that charts/ panels can go in the root directory.

I disagree that single files should replace utils/components. Instead, I'd rather utils start out as utils.js modules, and get "upgraded" to utils/ directories once they get large, in a way that doesn't lead to import refactors.

Stephen-ONeil commented 3 years ago

Archived doc draft from closed PR, to revisit when the repo is less active and a full directory reorg is more feasible. https://github.com/TBS-EACPD/infobase/blob/4efbe04269749a86da404f3aa431db465369dff4/docs/client/directory-structure.md