Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor project structure due to increasing number of code lines per page #339

Closed
joeldavidw opened this issue Oct 5, 2021 · 3 comments · Fixed by #740
Closed

Refactor project structure due to increasing number of code lines per page #339

joeldavidw opened this issue Oct 5, 2021 · 3 comments · Fixed by #740
Assignees
Labels
area/core area/workflow kind/refactor Non feature refactor change triage/accepted Triage has been accepted

Comments

@joeldavidw
Copy link
Contributor

What would you like to be added:

Pages are starting to grow larger and is exceeding 250 lines of code.

Why is this needed:

In order, to ensure maintainability of our code base due to increasing number of code lines. We should move internal components out of their respective pages.

To ensure separation of concerns, these internal components should be moved in to a folder called _components in their corresponding folders under pages/* dir instead of the src/components.

Similarly, to constants.ts not allowed, by placing components that are only of concern to a single page into src/components will result in a mass of code concentration within project.

Example for Blocks Page

src/
└─ pages/
  └─ blocks/
     └─ [blockId]/
        ├─ _components/
        │  ├─ BlockDetailTable.tsx
        │  └─ BlockTransactions.tsx
        └─ index.tsx

/triage accepted
/area workflow
/kind refactor

@defichain-bot defichain-bot added triage/accepted Triage has been accepted kind/refactor Non feature refactor change area/workflow labels Oct 5, 2021
@fuxingloh
Copy link
Contributor

Make sense, good to have concerns with close context and responsibility closely together than having them separated by functionality. The DeFiCh/wallet repo is following a similar design.

You can create and update the eslint rules to prevent export default on pages/*/_components .

/assign @joeldavidw
/area core

@aikchun
Copy link
Contributor

aikchun commented Oct 6, 2021

Wouldn't the folder structure under pages create their own routes?

Not having export default on those components will throw an export error for those routes.

Maybe we can recreate the pages folder structure in src/components instead, which we already sorta kinda do?

The reusable components in various pages can be placed in src/components/commons.

@joeldavidw
Copy link
Contributor Author

Thanks @aikchun for heads up! Yep, I think we should go with what @aikchun has said and recreate the pages folder structure in src/components.

If src/components ever gets too large and unwieldy in the future we may look into solution for the export default error posted in vercel/next.js#8617 or when next.js provides more support for this.

@joeldavidw joeldavidw added this to the Pre Vaults milestone Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/workflow kind/refactor Non feature refactor change triage/accepted Triage has been accepted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants