Skip to content
This repository has been archived by the owner on Jan 31, 2023. It is now read-only.

[RFC] Proposal for new file structure #8

Conversation

madsnedergaard
Copy link
Member

@madsnedergaard madsnedergaard commented Oct 28, 2022

This is a proposal for a slightly different file structure. I'd love you input and feedback, @tonypls and @Kongkille :)

(Note that this conflicts with other PRs, so for now this is just a PR to discuss things)

Frame 1 (1)

Follow-up questions

  • Nesting - yay or nay? E.g. "LeftPanel > Zone Details" vs flat

@madsnedergaard madsnedergaard force-pushed the madsnedergaard/ele-1337-set-up-feature-based-file-structure branch from 6b7fe6b to c00fdbe Compare October 31, 2022 10:06
@madsnedergaard madsnedergaard changed the base branch from mk/add-tanstack to master October 31, 2022 10:07
@tonypls
Copy link

tonypls commented Oct 31, 2022

I'd prefer to use map.tsx over index.tsx, it makes navigating to files on vscode easier and I think it's clearer to read. To clarify this we can make the file that matches the directory name is always the entry. i.e. Map > Map.tsx is the root component for feature directories.

I'm not sure about sub directories called components within a feature, similar with the index confusions I think it's nicer to use more descriptive naming here and avoid having multiple directories called components scattered around the app. I would suggest using the nested style of Left Panel > Zone Details or Map > Map Controls. we could aim to keep things flat until nesting makes sense.

@madsnedergaard
Copy link
Member Author

I'd prefer to use map.tsx over index.tsx, it makes navigating to files on vscode easier and I think it's clearer to read. To clarify this we can make the file that matches the directory name is always the entry. i.e. Map > Map.tsx is the root component for feature directories.

Yeah, that does make good sense - let's do that :)

I'm not sure about sub directories called components within a feature, similar with the index confusions I think it's nicer to use more descriptive naming here and avoid having multiple directories called components scattered around the app.

I see your point, I was just worried that one feature folder would suddenly have 10+ files which might make it harder to get an overview and navigate... But we can also deal with that when we get there 😄

I would suggest using the nested style of Left Panel > Zone Details or Map > Map Controls. we could aim to keep things flat until nesting makes sense.

Good point, I think it would also make it more clear how the components are used (e.g. an outer LeftPanel "wrapper" that hosts the different panels inside). Let's do as you suggest and aim for flat with exceptions 👍

@tonypls
Copy link

tonypls commented Oct 31, 2022

Okay sounds good, let's revisit at some point!

@VIKTORVAV99
Copy link
Member

I'm not sure if this what @tonypls meant but I'd like to see that every component has it's own folder that contains both the "index" file with the same name as the folder Map (folder name) > map.tsx (component name).
But I'd also like to see that we put component tests in the same folder as the component itself, so the Map folder would now contain map.tsx and map.tests.tsx (or map.spec.tex). This combined with vscodes File Nesting makes it very easy to navigate component trees with spec files.

Components
├─ Map
|  ├─ map.tsx
|  └─ map.tests.tex
└─ "Other Component"

What do you guys think?

@madsnedergaard
Copy link
Member Author

I'm not sure if this what @tonypls meant but I'd like to see that every component has it's own folder that contains both the "index" file with the same name as the folder Map (folder name) > map.tsx (component name).
But I'd also like to see that we put component tests in the same folder as the component itself, so the Map folder would now contain map.tsx and map.tests.tsx (or map.spec.tex). This combined with vscodes File Nesting makes it very easy to navigate component trees with spec files.

I agree on co-locating tests as X.test.tsx instead of inside tests folders :)
Re: VSCode file nesting I found this great setup for a lot of groupings - some of them are very opinionated, but I like it so far: https://github.com/antfu/vscode-file-nesting-config

Regarding everything component in it's own folder I'm not sure if we really want to go that far. I personally don't think we should aim for having unit tests for every single component (at least not in the beginning), so there might be components with just one file related to it.

I think we should try it out for a while and then group things when it feels natural, and then see if we can setup automated rules for organizing based on those early experiences :)

@VIKTORVAV99
Copy link
Member

I agree that it will probably be a too big of a underatking to get every single component its own unit tests right now, but we should plan for the future and a long term goal should be to have a complete test coverage when possible.

Regarding the folders itself it's mostly a personal choice at this point since we don't have any additional files for now.

That config you found is huge!
I wasn't planning on taking it that far but it looks good. We should also add some "Electricity Maps" custom nesting rules like nesting subzone config folder under the country level file, and do the same for the exchanges. I'll see if I can get some rules working locally then I can post them here.

@VIKTORVAV99
Copy link
Member

This should do the trick for the zones:

  "explorer.fileNesting.patterns": {
    "*.yaml": "${capture}-*.yaml"
  }

But since there is only one capture group ther is no reliable way to do it for the exchanges.

@madsnedergaard
Copy link
Member Author

That config you found is huge! I wasn't planning on taking it that far but it looks good.
Yeah, but there's an extension that handles it and keeps it updated 😄
https://github.com/antfu/vscode-file-nesting-config/tree/main/extension

We should also add some "Electricity Maps" custom nesting rules like nesting subzone config folder under the country level file, and do the same for the exchanges. I'll see if I can get some rules working locally then I can post them here.

Good point, I've taken the js/ts/tsx patterns from the repo I shared + your pattern and added it all to.vscode/settings.json along with enabling the feature :)

@madsnedergaard madsnedergaard merged commit 33344b4 into master Oct 31, 2022
@madsnedergaard madsnedergaard deleted the madsnedergaard/ele-1337-set-up-feature-based-file-structure branch October 31, 2022 22:07
@madsnedergaard
Copy link
Member Author

Whoops, thought it was reviewed already... Oh well, let me know if there's any parts we should revert 😚

@VIKTORVAV99
Copy link
Member

Shouldn't the .vscode folder be on the top level outside web?
Some of the setting should be applied to all the code not just the web folder (like my rule as it applies to the config folder... 😅), we could also add the recomended extensions like pylance, python, prettier, isort, black, eslint, yaml as well then.

@madsnedergaard
Copy link
Member Author

Shouldn't the .vscode folder be on the top level outside web? Some of the setting should be applied to all the code not just the web folder (like my rule as it applies to the config folder... 😅), we could also add the recomended extensions like pylance, python, prettier, isort, black, eslint, yaml as well then.

There's a symlink - so it works either way right now ¯\_(ツ)_/¯
(Although for the extensions that might actually be a problem, as we want contributors to be able to work on only frontend OR parsers, so they maybe shouldn't be shared... I just like being able to open both / and /web in VSCode and still have things working :D)

@VIKTORVAV99
Copy link
Member

Shouldn't the .vscode folder be on the top level outside web? Some of the setting should be applied to all the code not just the web folder (like my rule as it applies to the config folder... 😅), we could also add the recomended extensions like pylance, python, prettier, isort, black, eslint, yaml as well then.

There's a symlink - so it works either way right now ¯_(ツ)_/¯ (Although for the extensions that might actually be a problem, as we want contributors to be able to work on only frontend OR parsers, so they maybe shouldn't be shared... I just like being able to open both / and /web in VSCode and still have things working :D)

Symlinks don't work on windows 🙈.
I always open up the full repo as there is so much stuff interconnected between the config and web folders anyway (and then open multiple terminals).

But this is highly individual and I'm not sure we should override user setting too much.

@madsnedergaard
Copy link
Member Author

Symlinks don't work on windows 🙈.
Oh, didn't know that 😅

I'm hesitant though on recommending users to install a TailwindCSS extension if they're just changing a parser 🤔

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants