Repository navigation
[RFC] - Accommodating code to render for apps inside dotcom-rendering/ #7174
Description
Activity
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
apptoapps? - 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.tsxI 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,appsandwebfolders, as in your original example. We could have most components at the root, with small folders ofapps,web,editionsandampcomponents. Or we could have all components at the root, with variants under some of them for apps, web, editions etc.- Could we rename
@JamieB-gu thanks so much, these are all really great points. I have renamed
app=>apps, and addededitionsin 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.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/andlib/, 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?
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 ...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 ofsrc/:src/ apps/ web/ ...This means
apps/will import a lot fromweb/. 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.
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.tsxHere,
Button.tsx,Header.tsxandArticle.immersive.tsxare 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 inlibsshould be as library-like as possible.That is, things in
applicationshandle business logic (e.g. different data models) and compose things fromlibs, while things inlibsdo not really know a lot about the things that compose them.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/webas the root and just imagine it is./src, so that the refactor should be more seamless.e.g.
- create
./src/web/server/apps/endpoint.ts - create
./src/web/components/apps/Layout.ts - Start rendering out from there
- 🏁 Checkpoint to see if it's working
- If so - Move
webrendering target into it's own directories.src/web/web/- remove
./webdirectory
- remove
- If not, find another approach, none of the code will be lost, just relocated
- If so - Move
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.
- locked and limited conversation to collaborators
on Apr 23, 2024
[RFC] - Accommodating code for apps rendering inside
dotcom-rendering/🎉 The DC-A-R project is starting! DCR will render articles for an additional platform: the mobile apps. This is in addition to the existing platforms,
ampandweb.To accommodate this new platform, we need somewhere to put code specific to rendering for apps. We might also need to move code which is shared between platforms, especially as it's likely we will be able to use many of DCR's web components for apps as well.
Before thinking about where to put new code or move existing code to, there are some questions to consider (and possibly more not listed here!):
A starting point for discussion
In the "apps rendering in DCR" spike, we created an app sdirectory to sit alongside
web/andamp/:This helped keep app-specific code separate, but led to apps layouts importing a lot from
web's components. This coupling wasn't very clear when working from theweb/directory, but it didn't create any significant problems during the spike.The main idea of this starting point: create an
apps/directory for apps-specific code and aneditions/directory, and move things which are shared by multiple platforms into asrc/common/directory. This could give a structure that looks something like:This is just a starting point for discussion. I'm curious to hear any thoughts, alternatives or concerns!
Additional questions
web/to something, and nestapp/in there?