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

Gut the project aggressively, down to bare bones #86

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

curran
Copy link

@curran curran commented Jun 21, 2018

Greetings @iaincollins and thank you for this amazing project!

This PR is not intended to be merged (feel free to close immediately), but rather is intended to document and communicate my experience bootstrapping a project from nextjs-starter. Also documented in this Medium post.

This starter codebase has excellent authentication flows. This is what I was looking for. However, this codebase has a lot of things I'd rather not see in a starter example (especially one that I plan to actually use to bootstrap a long-term project), such as nuanced UI design and dependencies on packages that are really extraneous to the goal of a simple authentication example, such as Bootstrap, Reactstrap, and a host of others.

I'm not sure what could become of this PR, but if anyone wants to take it further, to gut it more and clean it up, this might be the sort of thing that would be possible to include as a Next.js example, in the Next.js repository, addressing the original issue vercel/next.js#153 .

@bennygenel
Copy link
Contributor

bennygenel commented Jun 22, 2018

I don't want to be seem like advocate of @iaincollins but I would like to share my thoughts on this topic.

You have pretty good points on how this project has some unnecessary parts ( like react-syntax-highlighter) that most of the users won't need in development/production but I'm not sure if this project is tend to use out of the box. This project is an example for showing how things work when the project gets a little bit more complicated than a Hello World or a Todo example. Those examples are everywhere and tend to show how easy things with Next.js.

From README.md

This is a starter project that provides an example of how to use Next.js with Express (the popular web server framework for Node.js), with SCSS, Bootstrap, reactstrap (Boostrap 4 for React), the Ionicons icon set, how to include data from remote REST APIs and incorporates an authentication system that supports both oAuth and Email using Passport (a popular authentication framework for Node.js).

If you start to plan your projects (like Auth) things start to get complicated pretty quickly. This project shows how can you plan out your own project. Rather than a bootstrap project, this is a Kitchen Sink where he shows some framework structure.

If you need a starter project using Nex.js with Auth you should have started with next-auth example. It's a bare minimum application that has Auth flow. I think there should be a with-next-auth example in zeit/next.js

@iaincollins
Copy link
Owner

Thank you for the kind words and sorry for not replying sooner (I've been on an extended holiday).

I think this is great and very much appreciated, as are the counter arguments.

I'll respond later (probably not for 2-3 weeks yet) but as a quick response, I think the case that Reactstrap (and all the stuff that goes with it) is possibly too opinionated and bloats it out too much with a bunch of dependancies, and certainly the SASS needs refactoring and there is scope to slim it down a lot – and the case for doing so is well made.

It's really handy to have a simple project which shows how to get SASS up and running quickly but I appreciate it's a pig of a dependancy as it needs compiling as a native binary. Newer versions of the SASS spec are also changing in annoying ways which make it harder to work with, which further complicates things so I've had it in mind recently so it needs some sort of resolution soon.

I'm probably not sold on going all the way to ditching Bootstrap yet, even if it opinionated - unless other folks think that it should go too (I'm happy to go with consensus). It's nice to have something where you can going quickly without having to figure out the step of how to include something like it.

I feel this way about an icon library by default too, even if folks will want to pick alternatives for both.

Certainly the project doesn't need complicated CSS so maybe an example that just uses custom local CSS and shows how you could include a library like Bootstrap (e.g. by uncommenting a line of SCSS) would be enough.

Comments and feedback always welcome!

@curran
Copy link
Author

curran commented Jul 19, 2018

Thanks @iaincollins for your response! And indeed for the project, it really helped me get off the ground, and the gutted variant has become the foundation for an ongoing project.

FWIW, I migrated to Bulma as it provided a nice way of including only the required modules, and not the whole library. Maybe one line of investigation could be to drop the wholesale inclusion of the entire Bootstrap default bundle, and replace it with a build step that bundles only the Bootstrap modules required.

I don't think SASS itself is necessarily a bad dependency, it's just that the way CSS in general is handled in nextjs-starter is not ideal, as it inlines it with the HTML that comes from the server. This may be time-related, meaning that I think when nextjs-starter was created, it was probably the best solution at the time, but since then new packages have popped up. In this regard, I ended up migrating to a combination of next-css and next-sass, which bundle the CSS and SASS nicely into a single file.

@iaincollins
Copy link
Owner

Update: This ticket is not dead, and will be looking at it again soon.

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.

3 participants