SIP-61 src/pages structure proposal

[EugeneTorap]

SIP-61 provides a new structure for our frontend folders.
In this discussion, we will only touch on structure of the pages.

1. We plan to move all src & src/views/CRUD pages to src/pages
2. Also need to fix folder naming inconsistencies and use camelCase for every folder name.
3. It's also a good idea to group creation and list page in one folder. For example: chart has two pages: creation and list
4. Make the folder nomenclature consistent in terms of using the plural form. Only sqlLab, explore, and home should be kept in the singular form.
5. We plan to take the iterative approach and move the pages one by one in order to reduce the chance of breaking current PRs
6. Rename welcome to home

Before

src
   addSlice
   dashboard
   explore
   profile
   SqlLab
   views
      CRUD
         alert
         annotation
         annotationlayers
         chart
         csstemplates
         dashboard
         data 
            database
            dataset
            query
            savedquery
         welcome

After

src/pages
   AlertReportList
   AnnotationLayerList
   AnnotationList
   ChartCreation (AddSlice)
   ChartList
   Chart (ExplorePage)
   CssTemplateList
   DashboardList
   Dashboard (DashboardRoute)
   DatabaseList
   DatasetCreation (AddDataset)
   DatasetList
   Dataset
   ExecutionLogList
   Home
   QueryHistoryList (QueryList)
   RowLevelSecurityList
   SavedQueryList
   SqlLab (don't move it until it's integrated into SPA)

[EugeneTorap]

My first PR for this proposal - chore: Move charts to src/pages folder 🙌

[EugeneTorap]

Maybe it's better to rename the welcome page to the home page?

[michael-s-molina]

I'm fine with this change.

[EugeneTorap]

Updated content.

[EugeneTorap]

We are planning to take the iterative approach and move the pages one by one in order to reduce the chance of breaking current PRs

[michael-s-molina]

Agreed. Can you add that to the proposal? Let's continue editing its content while we're discussing it.

[michael-s-molina]

Thank you @EugeneTorap for the discussion. I'm pinging some folks to help with the discussion and also to raise awareness @kgabryje @geido @codyml @eschutho @lyndsiWilliams

@EugeneTorap Some first-pass comments:

• Let's make the folder nomenclature consistent in terms of using the plural form. Only sqlLab, explore, and home should be kept in the singular form
• @lyndsiWilliams @eschutho Is src/views/CRUD/data/query/QueryList still used?
• I think we can simplify the package names and avoid repeating its context like charts/creation instead of charts/chartsCreation. WDYT?

[EugeneTorap]

1. and 3) - I absolutely agree and updated discussion content.

[eschutho]

Is src/views/CRUD/data/query/QueryList still used?

yes, it is

[EugeneTorap]

I suggest to rename queries to queryHistories in order to make the page name on UI side and folder name consistent.

[michael-s-molina]

I think it would be queryHistory. What do you think @eschutho?

[eschutho]

If we try to match our folder structure to page names, then what happens if a page moves or is renamed?

[codyml]

This looks great to me. One question: it looks like the above schema combines src/dashboard, which is the dashboard single view, with src/views/CRUD/dashboard, which is the dashboard list/creation views. Should these follow the pattern of src/pages/explore (chart single view) and src/pages/charts (chart list/creation views) being separate top-level items and have a separate src/pages/dashboard (single) and src/pages/dashboards (list/creation)? Or, should we start a new pattern of nesting the single view for object types within the plural-named folder? E.g. src/dashboard --> src/pages/dashboards/single and src/explore --> src/pages/charts/single? Not tied to any of the example names above, more wondering about the hierarchy.

[EugeneTorap]

It's the good question! @michael-s-molina what do you think about it?

[eschutho]

Thanks @EugeneTorap for the SIP! Similar to my comment above, what defines a page? Is it just a component that is linked to a route? There's nothing really intrinsic about the component itself that makes it a page, but rather how it is used. The way that I see it now is that we have two types of components: design components and everything else. Some components like the database modal can appear on any page, so where do they go? I don't know that I would consider that a page, or even nest it under the database page folder. Similarly, the dataset modal is going to be a full page soon. I'm not sure if we should be binding our folder nomenclature that strongly to how the component is used.

I'd be curious to hear @eric-briscoe's thoughts, too, since he's working on the design components.

[michael-s-molina]

Thanks for your comments @codyml @eschutho. They were really valuable to the discussion.

I went back and reviewed SIP-61’s references and also searched for new ones such as How To Structure React Projects From Beginner To Advanced and Evolution of a React folder structure and why to group by features right away and a possible structure could be:

src/components (may be moved to an external lib)

src/features
   alerts
   annotations
   annotationLayers
   charts
   cssTemplates
   dashboards
   databases
   datasets
      DatasetEditModal
      DatasetList
      DatasetPicker
   profiles
   queries

src/pages
   AlertReportList
   AnnotationLayerList
   AnnotationList
   ChartCreation (AddSlice)
   ChartList
   Chart (ExplorePage)
   CssTemplateList
   DashboardList
   Dashboard (DashboardRoute)
   DatabaseList
   DatasetCreation (AddDataset)
   DatasetList
   Dataset
   ExecutionLogList
   Home
   QueryHistoryList (QueryList)
   SavedQueryList
   SqlLab (don't move it until it's integrated into SPA)

Features are reusable parts of pages. They are not atomic UI elements, those are the components. Think of features as Superset specific parts while components could be reused by any other application. A feature may be promoted to a component if it’s reusable outside of Superset. Some examples of features are:

• The dataset edit modal
• The visualizations gallery
• The dashboard grid

The names of the feature folders represent the domain and not where a particular feature is used. Using the datasets folder as an example, it would contain dataset-related features such as a dataset edit modal, a dataset list, a dataset picker, etc.

Pages are a composition of features and they are mapped to the navigation routes of the application. Two pages can use the same feature such as the dataset edit modal.

Because we didn’t have these concepts before, some folders or component names may seem weird at this stage, but as we continue to reuse the features across pages, the concepts, dependencies, and names will improve over time. The same applies to our actions and reducers, they are all grouped now but over time we’ll have them split by feature. WDYT?

[eschutho]

That sounds good. I like the separation, and don't have strong opinions on the nomenclature, but I think what you propose could work.

[codyml]

With the latest updates this looks great to me! One thing I was thinking about was how to decide how much logic and layers of component definition should go in pages components vs. in features components. One easy rule that could be lint-enforced would be that you should never have to import anything into a pages component from another pages component; if you're doing that you should move the offending import into features.

[michael-s-molina]

Hi @EugeneTorap. We received great feedback from @eschutho @codyml and @eric-briscoe and kept the discussion open for more than a month to give the opportunity for more contributions. We have reached a good structure and we're ready to incrementally move the files. Thank you all for the discussion.

[michael-s-molina]

#23241 implements the changes outlined here.