-
Notifications
You must be signed in to change notification settings - Fork 154
[Proposal] next.js support #59
Comments
Yeah, I like the idea of supporting Next.js as well! Thanks for creating the issue. It's a mid-level priority to me; not something I feel is critical, but not a long-term vision either. I'm a tiny bit wary of the added complexity cost for adding it, although I think it's also a good opportunity to clean up some of the less-than-stellar stuff with Gatsby (effectively I want to avoid sprinkling project-specific conditionals across the codebase, and find a cleaner way to differentiate between projects).
Are you on Linux or Windows? If you're on Linux, it actually seems to run fine (see discussion in #45, we'll have a release soon to officially support building for it). Windows is a bit of a larger challenge, but that's coming soon as well (discussion tracked in #27) |
@joshwcomeau I'm on a Windows machine but good thing that I'm working on Windows support. So hopefully soon it is not going to be a big problem =)
I agree that this addition would push the project on a cleaner state. If we implement the structure you mention on #63 (comment) ( After road-map for this gets clearer I'll gladly work on it. |
Yep! Agreed. Excited to see support coming in the future :) |
@joshwcomeau I'm working on this. I haven't pushed any thing yet. Next.js it's working. I also think that we need to refactor the task execution as it feels difficult to add new things because of editing multiple locations and as mentioned sprinkling some conditionals here and there. So to be more precise the following needs to be refactored (we also need to discuss these points as there are probably multiple ways to refactor):
I think we could create a folder in
These file will return a configuration object e.g. for CRA: export default {
devServer: {
taskName: 'start',
args: ['run', 'start'],
env: (port) => ({
PORT: port
}),
create: {
// not sure if we need that nesting but I think there could be more to configure
args: (projectPath) => [
// used for project creation previous getBuildInstructions
'create-react-app',
projectPath
]
}
} Usage for export const getDevServerCommand = (
task: Task,
projectType: ProjectType,
port: string
) => {
const config = require(`src/project-type-configuration/${projectType}`);
return {
args: config.args,
env: config.env(port)
};
} And for export const isDevServerTask = (name: string, projectType: ProjectType) => {
const config = require(`src/project-type-configuration/${projectType}`);
return name === config.devServer.taskName;
} Usage in getBuildInstructions in export const getBuildInstructions = (
projectType: ProjectType,
projectPath: string
) => {
// For Windows Support
// Windows tries to run command as a script rather than on a cmd
// To force it we add *.cmd to the commands
const command = formatCommandForPlatform('npx');
const config = require(`src/project-type-configuration/${projectType}`);
return [
command,
...config.create.args(projectPath)
];
} I think this could work. I just don't like the dynamic requires that much but I think it's easier to add new project types. But it would be also OK to create one file with the configuration for each project and then we only need to import that file Another point that's looking not that clean are the functions inside the configuration but that's probably OK. I'll check later if we could add some sort of config variables e.g. Please let me know if that's a way to go or if we should do that differently. Is it OK to refactor and add Next.js with one PR? |
I think next.js can be really good addition to this program since it also supports creating projects with
npx
oryarn create
with lots of example projects.For someone just started using React, setting up SSR can be really confusing so next.js's out of the box support can be really helpful to get things started.
I already read about the possible addition on "How it works" part of the readme but thought it might benefit from getting its own issue to track down and discuss.
I would very much like to work on this but since I don't have a MacOS system I might not be able to test anything. Other than that if someone has any ideas on me how to contribute I am open for suggestions.
Thank you for the great project.
The text was updated successfully, but these errors were encountered: