Skip to content

Code cleanup#969

Merged
shcheklein merged 10 commits into
masterfrom
code-cleanup
Feb 4, 2020
Merged

Code cleanup#969
shcheklein merged 10 commits into
masterfrom
code-cleanup

Conversation

@iAdramelk
Copy link
Copy Markdown
Contributor

We are preparing to migrate from nextjs to gatsby, and to make migration easier, I made some code cleanup. Cleanup goals were to make folder structure more comfortable to use with gatsby, separate next-specific code from react components and remove as much next.js stuff that we didn't need as possible.

There is a lot of changes. To make it easier to review and read, I tried to group different tasks to different commits, so it will probably be easier to read it commit by commit then to read full diff.

What have been done:

  • 7748c86 Move react components to components folder. Gatsby stored pages in the src/pages folder, but we are storing our components here too. To make it easier to navigate and distinguish between them, I moved components to the subfolder src/components.
  • fa371db and 7bf941f Consistently rename components and styles files. Most of our components are named Component/index.js, but some older components are named Component/Component.js, I renamed all such components to the newer format.
  • 9258f52, 57b33bf, 81cdee8, 50beb01 - I split pages to the two separate components - one responsible for render and visual styles (moved to src/components) and one responsible for routing, header data and getting initial props from API (left in the pages folder). Because I will need to refactor pages logic from nextjs to gatsby, it will be easier to do if there is no presentational logic in them.
  • 59e399d Move styled-components to separate files. After we migrate to gatsby, we wouldn't need styled-components anymore. To make replacing them easier in the future, I moved them all to separate files with the same logic as was used on the community page.
  • 74c6597 Refactor NextLink and NextRouter. Here I create a standard link component and use it instead of the next/link in our files; this way, we will only need to replace for gatsby-link in one place. Also, here I removed some cases of next/router usage that we don't need.

@shcheklein shcheklein temporarily deployed to dvc-landing-code-cleanu-q3jez7 February 4, 2020 07:03 Inactive
@shcheklein shcheklein merged commit 484ae8a into master Feb 4, 2020
@jorgeorpinel
Copy link
Copy Markdown
Contributor

Thanks @iAdramelk. Massive but greatly explained PR!

Move styled-components to separate files

Nice. Should we also rename all the styles.js files to styled.js (per a conversation had some time ago). (Never mind, see next point.)
Also, do you think the issues below are still valid now?

After we migrate to gatsby, we wouldn't need styled-components anymore

Let's try remembering to close css: migrate from styled comps to plan css or css-modules #792 when that happens.

Thanks!

@iAdramelk
Copy link
Copy Markdown
Contributor Author

@jorgeorpinel Thanks! I think that #658 and #777 are still valid, but I'd place them on hold for now, before we finish with Gatsby and decide where to migrate styles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants