Update folder structure

[rin-st]

• Update components to

  Components
  +-- Ui
  |    +-- {simple universal components like button}
  +-- Page1
  |    +-- {components for Page1 only}
  +-- Page2
  |    +-- {components for Page2 only}
  +-- Shared
  |    +-- {shared complex components}

• Remove scaffold-eth folders from components, hooks, and utils.
• I believe we should have one file for one react component. So I think it will be better to remove utilsSomething.tsx files and replace them with files/folders

@carletex @technophile-04

[carletex]

I appreciate this @rin-st We should have this kind of discussions to keep iterating & improving.

Remove scaffold-eth folders from components, hooks, and utils.

The reason for it: One day, scaffold-eth components & hooks (which we can call "the core" of SE-2) might be a separate package (or packages) so people can use it in their dapps (e.g. @scaffold-eth/burner-wallet, @scaffold-eth/contract-debug, @scaffold-eth/eth-address-input). So want to keep it separate in the app.

We are building a tool for building dapps, it's not a final app, so people will be creating their own components / hooks as they need....and use ours (that for now live inside the project), need to be separated from theirs-

ExampleUI is a good example (:D) on how people would use SE-2, that's why I really like how you organized the code / folder structure on https://github.com/scaffold-eth/se-2/pull/115

I believe we should have one file for one react component. So I think it will be better to remove utilsSomething.tsx files and replace them with files/folders

I've seen both and I don't feel strong about any of the 2:

Option 1: +-- utils ++|---- network.ts (This file export a bunch of utils related to networks (getNetworkDetailsByChainId, getBlockExplorerTxLink, etc)

Option 2: +-- utils ++|---- network +++|------ getNetworkDetailsByChainId.ts +++|------ getBlockExplorerTxLink

I personally don't see a huge benefit in any of the two. For me it's just a matter of preference / style.

[rin-st]

The reason for it: One day, scaffold-eth components & hooks (which we can call "the core" of SE-2) might be a separate package (or packages)

Good point. So it's everything except the example UI page/components?

I've seen both and I don't feel strong about any of the 2:

Completely agree, it's ok to use that options for utils. But I talk for example about utilsComponents.tsx, utilsContract.tsx and utilsDisplay.tsx. The first one should be TxValueInput.tsx, the second should be in the utils folder, third should be a folder with three files :). In my opinion, it's much easier to find components by name in file structure and not so easy to group them into one file

[technophile-04]

Thanks so much @rin-st really love the suggestions and it's a topic to be discussed for better DX, I agree with the points mentioned by Carlos.

Completely agree, it's ok to use that options for utils. But I talk for example about utilsComponents.tsx, utilsContract.tsx and utilsDisplay.tsx. The first one should be TxValueInput.tsx, the second should be in the utils folder, third should be a folder with three files :). In my opinion, it's much easier to find components by name in file structure and not so easy to group them into one file

Yes this is a nice suggestion, adding some of my views to it :

The first one should be TxValueInput.tsx

Yup I really agree on this

the second should be in the utils folder, third should be a folder with three files :). In my opinion, it's much easier to find components by name in file structure and not so easy to group them into one file

Don't know why I really get overwhelmed when I see deeply nested directories/files and it starts giving me the feeling of complexity, but same as Carlos mentioned I don't feel strong with any of the two (making a utils folder and adding three files or keeping the current structure). But for sure would love to explore this(folder and three files) option when we plan to create @scaffold package

Also would love to see if there might be some more improvements meaning now if we want to use useEthPrice we import it from import {useEthPrice } hooks/scaffold-eth this looks great even I like utils/scaffold-eth similar to them as @rin-st pointed about extracting TxnValueInput like that there might some useful gems inside components/scaffold/contract which we can be handy for developers.

PS : I think maybe we can wait a bit before drafting a PR for this maybe 2 weeks or 1 and 1/2 weeks before the beta launch....since will have a clearer picture of what we have to move / group together. Also these are just my view/opinions and no problem if we deviate a bit 🙌

[rin-st]

Thanks for the answers!

Don't know why I really get overwhelmed when I see deeply nested directories/files

Deeply nested is okay when files are easy to find. Assume there are a lot of files in a folder, and you need to find a file by name. For me, it's much easier to open utils and find files here because there are fewer files in that folder :)

hooks/scaffold-eth this looks great even I like utils/scaffold-eth

hooks better are in hooks

before the beta launch

wow, didn't know :)

and another question, why do you need to reexport index files?

[technophile-04]

Deeply nested is okay when files are easy to find. Assume there are a lot of files in a folder, and you need to find a file by name. For me, it's much easier to open utils and find files here because there are fewer files in that folder :)

Yes agreed 100%, was just thinking from a beginner's perspective but your suggestions also make complete sense and happy to go with either of the approaches.

hooks better are in hooks

Lol sorry my framing was wrong I mean to say just like utils/scaffold-eth and hooks/scaffold-eth we get some useful functions/components from components/scaffold-eth/contract to components/scaffold-eth

wow, didn't know :)

😂 please don't quote on me(I may be wrong) @carletex will be the best person for this

[carletex]

Thanks for this Shiv & Rinat! I think we agree on most of the stuff.

I think maybe we can wait a bit before drafting a PR for this maybe 2 weeks or 1 and 1/2 weeks before the beta launch...

lol yes. The idea is to be more loud about SE2 in ETH Denver! It's going to be super helpful for us having a bunch of smart people using it, who can give us feedback.

Until then, I think we should focus on having a SE2 end-to-end version working, even if it's not perfect.

As a general guideline: We all want SE2 to be perfect (clean / well documented / the right abstractions, etc), but we don't want to fall into the "premature optimization" trap. Let's work on adding all the functionality we think we need (for the core of SE2) and keep iterating as we go. Also we should plan to start building a bunch of things with SE2!! That's also a good way to see what's missing / what could be improved.

In any case, I think having this kind of discussions are super healthy for the project. Thank you!

[carletex]

I've just merged #124 and made some tweaks regarding this discussion.

Specially check: https://github.com/scaffold-eth/se-2/commit/27f7a13bb89220b850ec99d8416d3273534c23f9

Don't know why I really get overwhelmed when I see deeply nested directories/files and it starts giving me the feeling of complexity,

I tend to agree on this, although in big projects I think it's the way to go: it might feel complex at the beginning but the project can grow better.

In any case, for ExampleUI, I didn't like (always thinking on the perspective that we are building a starter kit and this an example for people to use, not a final app) that you had to navigate through components to see the use of the contract write hook,: ExampleUI > ContractInteraction > PurposeSetter. I inlined the last levels of components, and I think it's still pretty legible.

We could even handle some of states on the top component (ExampleUI), and pass them as props. But that might be too much.

---

I also changed the folder structure, since we had less components and I don't see the need for extra folders.

Before:

After:

Note: We've been using the pattern (inherited from SE1) "default" with ability to "name import". This enables the use of code-splitting (with lazy loading) which we are now using right now... but could be useful at some point?

What do you all think?

The most important thing is being consistent and being on the same page, but we should always be reconsidering this kind of stuff!

[rin-st]

Nice!

I tend to agree on this, although in big projects I think it's the way to go: it might feel complex at the beginning but the project can grow better.

I worked mostly on much bigger projects, so I made components smaller based on my prev experience :). I agree structure became simpler, and I hope the code is easier to read for newcomers.

A question I forgot to ask at the beginning, why do we need reexport index files?

[carletex]

A question I forgot to ask at the beginning, why do we need reexport index files?

I think this it what I mentioned on the "Note" in my previous comment:

Note: We've been using the pattern (inherited from SE1) "default" with ability to "name import". This enables the use of code-splitting (with lazy loading) which we are now using right now... but could be useful at some point?

It this what you meant?

[rin-st]

hmm, yep. Sorry, I missed named part. But why use named export if we already have a default?

[carletex]

But why use named export if we already have a default?

I'm guessing it's a matter of preference (imports look nicer?)

import { ContractData, ContractInteraction } from "~~/components/ExampleUi";

vs

import ContractInteraction from "~~/components/ExampleUi/ContractInteraction";
import ContractData from "~~/components/ExampleUi/ContractData";

Not sure if there is another reason for it. And not sure if it's worth creating the extra file.

I made the tweak to the ExampleUI to follow the convention we adopted (from SE1)... but we could reconsider and refactor!

[carletex]

Also, regarding code-splitting:

From: https://nextjs.org/learn/foundations/how-nextjs-works/code-splitting

It seems that NextJS handle code-splitting pretty well out of the box.

So "pretty" imports (option 1 in my previous comments) might be the only reason to use that pattern.

We are also using this option: https://github.com/scaffold-eth/se-2/blob/main/packages/nextjs/hooks/scaffold-eth/index.ts, which doesn't have any defaults (won't work with React.lazy, but still work with dynamic import). I like this option because you can define the components the ES6 way.

So maybe (1) refactor all the index.ts to this option.... or (2) remove all the index.ts for good (if we don't care about cleaner imports).

In any case, not a top priority. Even tho it should be easy to implement any of these options....

[rin-st]

Thank you for detailed response!

I'm voting for (2), because maybe imports look prettier, but

• you don't see a real path to the file on the imports
• you need to do additional actions when you, let's say, reviewing pr and want to check the imported file (find reexport file -> find your file here -> find real file, if no deeper reexports)
• it's not clear if we need to reexport files from the deeper level to the top because it's prettier (or not?). Basically, we can create reexport file for all the components, so all imports will be import {a,b,c,d} from 'components'. So we need rules for that
• sometimes files can be imported as default and sometimes as named. Our IDE's can do imports differently

I don't like default imports and always prefer named https://humanwhocodes.com/blog/2019/01/stop-using-default-exports-javascript-module/ (post from 2019 and now most of the things IDE's can resolve, but we always can make a mistake). But if you prefer defaults I believe it's better having only defaults than having both

[technophile-04]

NOTE : Feel free to skip and read "TLDR; of summary" in last section 🙌

Love the discussion, adding some of my views :

Currently, I see we are using barrels files (leaving ExampleUI for now) in :

1. components/scaffold-eth
2. hooks/scaffold-eth
3. utils/scaffold-eth

Considering we have the plan to put all this in a package in the future, I think I love the current approach of having barrels files for now.

Reson : Seeing it from a perspective of using se-2 for making daap, I will be very less interested in its internal work(even using scaffold-eth v1 I don't know most of the things internally working).

So while using se-2 see imports as :

+ import { useScaffoldContractWrite} from "~~/hooks/scaffold-eth"; 
// insted of 
- import useScaffoldContractWrite from '~~/hooks/scaffold-eth/useScaffoldContractWrite'

Like the first one makes it declarative instead of pointing out the full location of the file.

---

• you don't see a real path to the file on the imports

I think this is an advantage of using barrels files 😅

• you need to do additional actions when you, let's say, reviewing pr and want to check the imported file (find reexport file - find your file here -> find real file, if no deeper reexports)

Yup 100% agree this might be frustrating for reasons mentioned in point, but seeing it from the perspective of using se-2 i feel it's good(reson mentioned in above section).

• so all imports will be imported {a,b,c,d} from 'components'

😅 I tend to like differentiation stuff like components/scaffold-eth telling that you are using something from scaffold-eth

---

!!!! IMP !!!! : I did some good digging and found this issue on official Next.js repo https://github.com/vercel/next.js/issues/12557. Would highly reccomend to through the issue discussion but here's a TLDR; :

• It seems that in Next.js Tree Shaking dosen't work with Typescirpt barrel file which increases the bundel size of app.
• To use tree shaking with barrel file, I saw people doing "sideEffects": false(but this didn't work for all ), also people have shared some webpack config to handle.

Really sorry if I am missing something and again recommend to go through the issue discussion once https://github.com/vercel/next.js/issues/12557 😅

Summary :

• Currenlty I see we are using barrel files only for scaffold-eth stuff like component/scaffold-eth and for stuff other scaffold-eth we are importing it directly instread of using barrel file like import Header from "~~/components/Header", fotter, Spinner etc.
• Since we are planning to put all the stuff in future in a package it makes more sense to use barrel files would highly recommend to check out this comment mentioned in above issue: https://github.com/vercel/next.js/issues/12557#issuecomment-1309240211
• I personally like (1) which Carlos mentioned but it comes with a performance issue, but if we are able solve it using some of the references mentioned in https://github.com/vercel/next.js/issues/12557 then would love if we can go for barrel files at least for exports from */scaffold-eth but if its not the case then I think (2) is the only way and not too bad and has some advantage which Rinnat mentioned in the above comment.

TLDR; of summary Would love to keep barrels files for exports from scaffold-eth directories like component/scaffold-eth, utils/scaffold-eth etc and for all others we can use (2) considering that we find soluiton for https://github.com/vercel/next.js/issues/12557. if we don't find solution for https://github.com/vercel/next.js/issues/12557 then (2) is the only option

PS : I think this is not that important as Carlos mentioned but seems like a priority issue after the launch because I saw some projects faced issues while refactoring/removing the barrel file since their project grew a lot checkout : https://github.com/vercel/next.js/issues/12557#issuecomment-1146036908

[sverps]

I think we should create another package in the workspaces called core where we can migrate all the things we consider part of the core (i.e. components/scaffold-eth, hooks/scaffold-eth, utils/scaffold-eth). That way we can create a clean separation that will help in multiple ways:

• As maintainers, we'll be able to separate concerns better and we'll avoid the risk of importing non-core things into the core package
• As maintainers, it'll be easier to turn this package into a real npm package
• As scaffold-eth "dev-user", it'll be more obvious from the start what the core is, and what is custom stuff that can be tinkered with

[carletex]

Hey @sverps

Thanks for the great suggestions. Great way to align having this discussions.

I think I like the way it's now. Let me explain my mental model for it:

In the scale of: 0 messy code / poor organization / bloated / inefficient 10. pristine-clean-craftsmanship-(overengineered code? :d). A lot of abstractions & code Indirection

We prolly want to stay in the 6-8 range.

As a maintainers, we definitely want to have something that it's easy to maintain and iterate without doing hacks. BUT, our priority (as BG members, SE2 maintainers) is developer onboarding. Most of the people that are coming (will come) to SE2 are beginners / intermediate (prolly advanced users will wire their own stack).

So the priority pyramid should be like:

1. The "thing" (component, hook, code organization) work super smooth (no weird bugs in certain conditions, bad UX, etc) (priority 10)
2. It's easy to read / use / modify for beginners / intermediates dev (priority 8 - 10)
3. It's clean and easy to maintain. (priority 6 - 8)

Seeing where we are heading and the new refactors we are making, we have to be careful not to prioritize nº3 and hurt nº2. Of course, it's all about balance.

So, coming back to:

I think we should create another package in the workspaces called core where we can migrate all the things we consider part of the core (i.e. components/scaffold-eth, hooks/scaffold-eth, utils/scaffold-eth). That way we can create a clean separation that will help in multiple ways:

The pyramid will look like:

1. No changes here
2. Will hurt a bit people when using it. I've watched a bunch of people use SE-2 these days, and directories can be confusing. Also people sometimes want to modify the components.
3. Some (marginal?) gain here

As maintainers, it'll be easier to turn this package into a real npm package

I could see this happening at some point! We'll need to make the components as headless as possible. For now I think it's great to see how people use it, how they modify them, etc.... so we get more info on what we need.

---

Hope all this make sense. And let's find together the best balance! cc @technophile-04 @rin-st

[sverps]

After reading your points, I think you're probably right. I was thinking a bit too much about the option to create some sort of npm package from our 'core', which is not really a priority (and we might never get to it anyway).

I do think we should continue considering refactors such as #219, as I believe they will benefit both users as well as maintainers. Here are my thoughts:

Seeing where we are heading and the new refactors we are making, we have to be careful not to prioritize nº3 and hurt nº2. Of course, it's all about balance.

From my experience, point nº2 and nº3 are very coupled. A key part of maintainable code is a clean and intuitive code structure, combining what is logically related, separating things that have different responsibilities. For users, just as well as for maintainers, it should be intuitive to locate which piece of code is responsible for a certain thing of interest, which should be the primary goal of any refactor we make. I think I dislike unnecessary abstractions just as much as everyone else here :relaxed:

[carletex]

I feel we are in the same page! Just wanted to put that out there to explain how I see it. I think we should explain it in the contributors file to make it easier to align with new contributors.

I do think we should continue considering refactors such as https://github.com/scaffold-eth/se-2/issues/219, as I believe they will benefit both users as well as maintainers.

Haven't check it out yet (will do when ETH Denver finishes), but taking a quick look at it seems like we are making the code cleaner / more maintainable.... and NOT hurting the builders. So if it's a WIN / WIN situation.

From my experience, point nº2 and nº3 are very coupled. A key part of maintainable code is a clean and intuitive code structure, combining what is logically related, separating things that have different responsibilities. For users, just as well as for maintainers, it should be intuitive to locate which piece of code is responsible for a certain thing of interest, which should be the primary goal of any refactor we make. I think I dislike unnecessary abstractions just as much as everyone else here

Yeah, win / win => let's go for it. I only have an issue with it when it hurts DX (specially for dev that are not experts, when you go down the path of over-engineering / early optimizations.

---

Let's just keep this meta-discussion open when thinking about adding new features / refactors. And let's use the "pyramid" mental model to find the sweet spot.

[carletex]

Since we are on "refactor season" (lol) I'm opening the discussion about named imports (barrel) VS direct imports. Started here: https://github.com/scaffold-eth/se-2/issues/118#issuecomment-1415648824

Shiv wrote a nice overview here: https://github.com/scaffold-eth/se-2/issues/118#issuecomment-1416839168

Would also love to hear your take on this @sverps

[sverps]

I'm a fan of barrel files in combination with named exports. From the discussion, I understood that this could have negative implications on tree-shaking etc., so the question basically comes down to whether or not the minor advantages weigh up against the disadvantages.

Maybe we should start with testing if the issue would occur in our repo as well if we have some barrel file.