[RFC] - Accommodating code to render for apps inside dotcom-rendering/
#11240
Replies: 7 comments
-
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.
Assuming the file structure above, I have a couple of small suggestions:
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:
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?
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.
I think the storybook question mirrors the file structure question? We could have |
Beta Was this translation helpful? Give feedback.
-
@JamieB-gu thanks so much, these are all really great points. I have renamed I also like your alternative suggestions - especially to consider having an even flatter structure and assume modules are common by default.
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.
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 |
Beta Was this translation helpful? Give feedback.
-
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'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 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? |
Beta Was this translation helpful? Give feedback.
-
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
... |
Beta Was this translation helpful? Give feedback.
-
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
This means
We will look back to the suggestions here when we revisit the folder structure again, in a few sprints time. |
Beta Was this translation helpful? Give feedback.
-
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?
Here, 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 That is, things in |
Beta Was this translation helpful? Give feedback.
-
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
=>
If we have a larger scale refactor it feels considerate if it:
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 e.g.
💯 agree, I don't think this interferes with that at all. |
Beta Was this translation helpful? Give feedback.
-
[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,
amp
andweb
.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?Beta Was this translation helpful? Give feedback.
All reactions