Skip to content
This repository has been archived by the owner on Jul 19, 2022. It is now read-only.

Next.js migration #79

Draft
wants to merge 32 commits into
base: master
Choose a base branch
from
Draft

Next.js migration #79

wants to merge 32 commits into from

Conversation

lfades
Copy link

@lfades lfades commented Mar 10, 2020

Overall goals of this PR:

  • Replace Gatsby with Next.js
  • Remove most configuration files and reduce the overall complexity of the project
  • Create a page per path, instead of using a single page that imports all components
  • Remove the sitemap from the client side bundle, it shouldn't be required once all pages have been migrated.
  • Move all queries and data requirements to getStaticProps
  • Remove all the server side related code, and gatsby related configurations. This is not a replacement with Next.js configs because those are incredible minimal in comparison.

Stuff to do:

  • Move page by page using the new structure.
  • Remove no longer used code and configuration files.

@SachaG
Copy link
Member

SachaG commented Mar 10, 2020

Wow this is really cool! But I guess for a change of this magnitude we should probably talk and coordinate a bit more…? I'm not quite sure where to start with this to be honest, but I'll try to take a look at what you have so far next week :)

@SachaG
Copy link
Member

SachaG commented Mar 10, 2020

Btw:

Create a page per path, instead of using a single page that imports all components

Do you mean having one physical file per path? We actually started out like this and migrated to the current system when we realized that we were duplicating a lot of the code. With the current system all pages are made of blocks, and the blocks are listed in the sitemap. So we ended up not really needing "static" pages.

Sorry if I misunderstood!

P.S. also it's worth pointing out that we want to be able to reuse this repo for other, different surveys. So it's important to us to keep the structure as flexible as possible.

@lfades
Copy link
Author

lfades commented Mar 11, 2020

@SachaG Yes, one physical page per path, there shouldn't be any kind of code duplication thanks to the chunks management of Next.js, but the main reason for this change is to make the content of each page way simpler than it's today, I also want to eventually get rid of the sitemap yaml files, which are also available in the client and are quite heavy, or at least replace it with a minimal version of what it currently is.

Could this be used for a future survey: Definitely, the way you edit the content will be way simpler after this.

@SachaG
Copy link
Member

SachaG commented Mar 11, 2020

I also want to eventually get rid of the sitemap yaml files, which are also available in the client and are quite heavy, or at least replace it with a minimal version of what it currently is.

While I really appreciate you taking the time to help us improve this project, I feel like this amounts to you deciding to completely change the code architecture without really consulting us…? Unless you are doing this project as a personal experiment for your own benefit?

@lfades
Copy link
Author

lfades commented Mar 12, 2020

@SachaG Not at all, we can get into a call if you want to, I don't want to do something that feels wrong to you, I'm still far from migrating all pages so any architecture changes to this PR are very welcome.

The only thing that I have decided is to remove as much code from the client bundle and configurations as possible, if that doesn't sound good I will definitely not do that. Tell me what is very important for the project moving forward and I'll make sure to keep it.

@lfades
Copy link
Author

lfades commented Mar 12, 2020

As of my above comment, I'll stop working on the PR until you are 100% sold to it.

@SachaG
Copy link
Member

SachaG commented Mar 21, 2020

Sorry for the lack of response, I've been pretty busy this past week. We should definitely jump on a call before you move forward just to compare notes. Could you email me at info at sachagreif dot com?

@lfades
Copy link
Author

lfades commented Mar 24, 2020

@SachaG Nice, just sent an email 👍

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

Successfully merging this pull request may close these issues.

2 participants