Activity

1. sunnyclimber456

   added

   dotcom-rendering

   RFC

   on Feb 10, 2023
2. JamieB-gu commented on Feb 13, 2023

   

   JamieB-gu

   on Feb 13, 2023

   Contributor

   More actions

   Thanks for writing this up @georgeblahblah! I suppose a key consideration for this will be how much we want to refactor the existing DCR codebase? Do we want to keep changes limited for now to minimise the impact on others working on the project? Or are we happy to make a more drastic change? I would be interested to hear from the rest of the team on this.

   This could give a structure that looks something like:

   Assuming the file structure above, I have a couple of small suggestions:

   • Could we rename app to apps?
   • Could we add a directory for editions?

   I also have an alternative suggestion whereby we flatten the file structure and assume common code by default, with the occasional sub-directory for product-specific code:

   src/
       components/
           apps/
           web/
           Headline.tsx
           Standfirst.tsx
           ...
       server/
       client/
       contributions.ts
       decidePalette.ts
       ...
   

   This might prove useful to encourage consistency and common code, as opposed to siloing by product. That said, this would be a more significant change to the codebase, and it may be too early to consider something this major, as discussed above.

   There's another possibility for organising components, which I think we may have discussed during the spike?

   components/
       Metadata/
           index.tsx
           apps.tsx
           web.tsx
   

   I would be keen to hear people's views on any of the above. Ultimately, I think we want whichever makes the project easiest to reason about and maintain for most people.

   How do we want things to look in Storybook?

   I think the storybook question mirrors the file structure question? We could have common, apps and web folders, as in your original example. We could have most components at the root, with small folders of apps, web, editions and amp components. Or we could have all components at the root, with variants under some of them for apps, web, editions etc.
3. sunnyclimber456 commented on Feb 13, 2023

   

   sunnyclimber456

   on Feb 13, 2023

   ContributorAuthor

   More actions

   @JamieB-gu thanks so much, these are all really great points. I have renamed app => apps, and added editions in the examples.

   I also like your alternative suggestions - especially to consider having an even flatter structure and assume modules are common by default.

   I suppose a key consideration for this will be how much we want to refactor the existing DCR codebase?

   yes, that's something we should definitely keep in mind. Depending on the preferred option, it might be possible to move things as we touch them rather than all at once - as long as that doesn't create too much confusion itself. I think this is highly dependent on the structure we choose.

   Do we want to keep changes limited for now to minimise the impact on others working on the project? Or are we happy to make a more drastic change?

   I think this is a question for the wider team. I'm generally in favour of minimising initial impact. With that in mind, we could also follow the approach in the spike, whereby we imported from web/components. We can then evaluate which structures make the most sense, or solve the most problems.
4. OllysCoding commented on Feb 14, 2023

   

   OllysCoding

   on Feb 14, 2023

   Contributor

   More actions

   This is great @georgeblahblah :)

   I think this is a really interesting question, which is not helped by the fact that the current DCR structure is poorly defined. I think for this reason, we should consider this an opportunity to bring clarity to directory structure where there might not have been before.

   I also have an alternative suggestion whereby we flatten the file structure and assume common code by default, with the occasional sub-directory for product-specific code:

   I'm actually a big fan of this solution @JamieB-gu, I think this avoids the risk of over-abstraction. I think this would also be a good opportunity to take a look at the AMP directory (which has fallen behind web in some ways), and try and move it to help keep it more up-to-date with the rest of the project.

   I think we'd definitely need product specific directories for things like server/, client/ and lib/, e.g:

   src/
       components/
           apps/
           web/
           amp/  
           Headline.tsx
           Standfirst.tsx
           ...
       server/ # Could this be one dir? Maybe with the right file names, but I'm not sure what that would be
           amp/
               lib/ # Is this needed? Currently amp has its own lib dir for server, there's a root server/lib, but not a web/server/lib
           web/ 
               lib/
           apps/
               lib/
       client/
           # Can we get away without platform dirs here? No client code for AMP, will apps/ need to differ from web?
       lib/
            # Might we need /amp /apps & /web here? 
            contributions.ts
            decidePalette.ts
       types/
            # Do we want to keep a types dir? We've been gradually moving away from using index.d.ts for types that don't need to be global.
       ...

   ---

   One question I'd have about having the above structure would be whether it'd interfere with @jamesgorrie future ideas around separating DCR by purpose (e.g articles, fronts, etc). My initial thought it no, and that any form of better definition around file structure will enable us to make that kind of change better in the future, but perhaps the initial suggestion by @georgeblahblah would be better for this?
5. Georges-GNM commented on Feb 14, 2023

   

   Georges-GNM

   on Feb 14, 2023

   Last edited by Georges-GNM

   Contributor

   More actions

   Just my 2 cents, I like the common code approach as well - the way I see it, it feels like a continuation of common-rendering in a way; which made sense to me as an approach, but I guess ended up not being as beneficial as hoped with AR and DCR being too separate projects until now.

   I'd also suggest we can still separate by purpose further down the tree as needed, something like this I think would make sense and probably be helpful in finding things swiftly:

   src/
       components/
           apps/
           web/
              fronts/
                 Cardheadline.tsx
                 Frontcard.tsx
              articles/
                 ArticleBody.tsx
                 TableOfContents.tsx
           amp/  
           Headline.tsx
           Standfirst.tsx
           ...
6. sunnyclimber456 commented on Feb 15, 2023

   

   sunnyclimber456

   on Feb 15, 2023

   Last edited by sunnyclimber456

   ContributorAuthor

   More actions

   Thanks everyone for your fantastic suggestions so far.

   To make sure we can get started sooner rather than later, we will take the approach of "least changes" initially. We will leave most existing code as-is, and create an apps/ subdirectory of src/:

   src/
     apps/
     web/
     ...
   

   This means apps/ will import a lot from web/. There are some key benefits to this approach, although it is messier initially:

   • we can start soon, having a place to put apps-specific code
   • when we do come to refactor the folder structure, it will be clear if there are any additional inputs to the decision in addition to what we have considered here
   • as we start refactoring, it will be clear whether it is solving the problem or introducing other difficulties

   We will look back to the suggestions here when we revisit the folder structure again, in a few sprints time.
7. sndrs commented on Mar 2, 2023

   

   sndrs

   on Mar 2, 2023

   Last edited by sndrs

   Contributor

   More actions

   Apologies for butting in, but I missed this earlier and I love this kind of stuff :) do ignore this if its not helpful...

   What do you think about something along these lines?

   applications/
   ├── article.immersive.client.ts
   ├── article.immersive.server.ts
   ├── article.standard.client.ts
   ├── article.standard.server.ts
   ├── front.client.ts
   ├── front.server.ts
   libs/
   ├── components/
   │   ├── Button.tsx
   │   ├── Cmp.apps.tsx
   │   ├── Cmp.dotcom.tsx
   │   └── Header.tsx
   └── layouts/
       ├── Article.immersive.tsx
       ├── Article.standard.apps.tsx
       └── Article.standard.dotcom.tsx
   

   Here, Button.tsx, Header.tsx and Article.immersive.tsx are either used in both apps and dotcom, or only one of them (e.g. Header.tsx).

   I think it allows for common components where they're simple enough to not need alternate versions, and for alternate versions where necessary, while making it immediately clear to the first-time browser how to find things and what's happening in each case.

   The other thing this heavily borrows from is Nx's mental model of an applications/libs distinction. In this way of thinking, you put as much business logic as practical into applications, and things in libs should be as library-like as possible.

   That is, things in applications handle business logic (e.g. different data models) and compose things from libs, while things in libs do not really know a lot about the things that compose them.
8. jamesgorrie commented on Mar 2, 2023

   

   jamesgorrie

   on Mar 2, 2023

   Contributor

   More actions

   we flatten the file structure and assume common code by default

   This is great as it creates fewer categorisations which I don't feel we've defined as yet (see github.com//issues/7256 for example).

   It also addressed something we should ensure is the case, if things are coupled, they are explicitly so, maybe reframing your consideration @georgeblahblah

   How can we make it hard to forget that some changes affect multiple platforms

   =>

   How can we make it evident that a change will affect 1 or multiple rendering targets

   ---

   Do we want to keep changes limited for now to minimise the impact on others working on the project

   If we have a larger scale refactor it feels considerate if it:

   • does not disrupt others work
   • does not allow for two+ patterns of doing things, creating cognitive overload (the refactor should be a complete refactor or have an iterative plan)
   • does not change repeatedly needing someone to repeatedly relearn a new pattern
   • should try not to have a tonne of conflicts on people's current work

   These are in line with - "I want to deliver a thing, not learn your platform" and from the perspective of people delivering features on the platform.

   To this end, and that we're still exploring, @JamieB-gu's suggestion offers an easy path forward.

   We could make it lighter touch to allow for some validation and change on the way. If we maintain ./src/web as the root and just imagine it is ./src, so that the refactor should be more seamless.

   e.g.

   1. create ./src/web/server/apps/endpoint.ts
   2. create ./src/web/components/apps/Layout.ts
   3. Start rendering out from there
   4. 🏁 Checkpoint to see if it's working
      1. If so - Move web rendering target into it's own directories .src/web/web/
         1. remove ./web directory
      2. If not, find another approach, none of the code will be lost, just relocated

   ---

   the above structure would be whether it'd interfere with @jamesgorrie future ideas around separating DCR by purpose

   💯 agree, I don't think this interferes with that at all.
9. guardian

   locked and limited conversation to collaborators on Apr 23, 2024
10. alinaboghiu

    converted this issue into a discussion #11240 on Apr 23, 2024