Skip to content
This repository has been archived by the owner on Jun 17, 2021. It is now read-only.

Refactor project types & add Next.js #292

Merged
merged 13 commits into from
Nov 1, 2018
34 changes: 34 additions & 0 deletions src/assets/images/nextjs.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions src/components/CreateNewProjectWizard/MainPane.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import styled from 'styled-components';

import reactIconSrc from '../../assets/images/react-icon.svg';
import gatsbyIconSrc from '../../assets/images/gatsby_small.png';
import nextjsIconSrc from '../../assets/images/nextjs.svg';

import FormField from '../FormField';
import ProjectIconSelection from '../ProjectIconSelection';
Expand Down Expand Up @@ -79,6 +80,7 @@ class MainPane extends PureComponent<Props> {
isFocused={activeField === 'projectType'}
>
<ProjectTypeTogglesWrapper>
{/* Todo: Make it easier to add new flows - e.g. map over an array to generate the UI*/}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

<ButtonWithIcon
showStroke={projectType === 'create-react-app'}
icon={<ReactIcon src={reactIconSrc} />}
Expand All @@ -96,6 +98,14 @@ class MainPane extends PureComponent<Props> {
>
Gatsby
</ButtonWithIcon>
<Spacer inline size={10} />
<ButtonWithIcon
showStroke={projectType === 'nextjs'}
icon={<GatsbyIcon src={nextjsIconSrc} />}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since GatsbyIcon is getting re-used now on something non Gatsby, we should generalize it (and ReactIcon) and have width and height props:

<IconImage width={'...'} height={'...'} src={...} />
const IconImage = styled.img`
  width: ${props => props.width || '32px'};
  height: ${props => props.height || '32px'};
`;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, good catch. That was my mistake I should have created a NextjsIcon - I missed that point.
I think I'll do the mapping as mentioned in line 83. So we only have the IconImage and map over the configuration keys to render the project type selection icons.

onClick={() => this.updateProjectType('nextjs')}
>
Next.js
</ButtonWithIcon>
</ProjectTypeTogglesWrapper>
</FormField>
</FadeIn>
Expand Down
2 changes: 2 additions & 0 deletions src/config/app.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// app-wide settings (no user changable settings here)
module.exports = {
PACKAGE_MANAGER: 'yarn',
// Enable logging, if enabled all terminal responses are visible in the console (useful for debugging)
LOGGING: true,
};
54 changes: 54 additions & 0 deletions src/config/project-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// project type configuration
// used for
// - create project command args
// - devServer name mapping
//
export default {
'create-react-app': {
devServer: {
taskName: 'start',
args: ['run', 'start'],
env: {
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,
],
},
},
gatsby: {
devServer: {
taskName: 'develop',
// gatsby needs -p instead of env for port changing
args: ['run', 'develop', '-p', '$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
'gatsby',
'new',
projectPath, // todo replace later with config variables like $projectPath - so we can remove the function. Also check if it's getting complicated.
],
},
},
nextjs: {
devServer: {
taskName: 'dev',
args: ['run', 'dev', '-p', '$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
'github:awolf81/create-next-app', // later will be 'create-next-app' --> issue not filed yet
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps update the comment for 'github:awolf81/create-next-app' to reference the issue URL, for tracking?

projectPath,
],
},
},
};
25 changes: 12 additions & 13 deletions src/reducers/tasks.reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
CLEAR_CONSOLE,
RESET_ALL_STATE,
} from '../actions';
import projectConfigs from '../config/project-types';

import type { Action } from 'redux';
import type { Task, ProjectType } from '../types';
Expand Down Expand Up @@ -222,6 +223,10 @@ export default (state: State = initialState, action: Action = {}) => {
//
//
// Helpers
export const devServerTaskNames: Array<string> = Object.keys(
projectConfigs
).map(projectType => projectConfigs[projectType].devServer.taskName);

export const getTaskDescription = (name: string) => {
// NOTE: This information is currently derivable, and it's bad to store
// derivable data in the reducer... but, I expect soon this info will be
Expand Down Expand Up @@ -251,7 +256,7 @@ export const getTaskDescription = (name: string) => {

export const isDevServerTask = (name: string) =>
// Gatsby and create-react-app use different names for the same task.
name === 'start' || name === 'develop';
devServerTaskNames.indexOf(name) !== -1;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: add next.js to comment

// Gatsby, create-react-app and next.js use different names for the same task.

Also any reason not to use includes() here rather than indexOf()? I find it a lot more intuitive.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right. includes is better as it's more intuitive. I'll change the comment & the includes later today.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps “Each framework uses a different name for the task that starts the development server”? That way, it doesn’t have to be changed for each new framework.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@j-f1 Even better :)


// https://docs.npmjs.com/misc/scripts
const preExistingTasks = [
Expand Down Expand Up @@ -291,7 +296,7 @@ const getTaskType = name => {
// For a dev server, "running" is a successful status - it means there are
// no errors - while for a short-term task, "running" is essentially the same
// as "loading", it's a yellow-light kind of thing.
const sustainedTasks = ['start', 'develop'];
const sustainedTasks = devServerTaskNames; // We could have other tasks we could add - for now we also could use devServerTaskNames directly

return sustainedTasks.includes(name) ? 'sustained' : 'short-term';
};
Expand Down Expand Up @@ -370,16 +375,10 @@ export const getDevServerTaskForProjectId = (
projectType: ProjectType,
}
) => {
switch (props.projectType) {
case 'create-react-app': {
return state.tasks[props.projectId].start;
}

case 'gatsby': {
return state.tasks[props.projectId].develop;
}

default:
throw new Error('Unrecognized project type: ' + props.projectType);
const config = projectConfigs[props.projectType];
const { taskName } = config.devServer;
if (!config) {
throw new Error('Unrecognized project type: ' + props.projectType);
}
return state.tasks[props.projectId][taskName];
};
77 changes: 62 additions & 15 deletions src/sagas/task.saga.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
receiveDataFromTaskExecution,
loadDependencyInfoFromDisk,
} from '../actions';
import projectConfigs from '../config/project-types';
import { getProjectById } from '../reducers/projects.reducer';
import { getPathForProjectId } from '../reducers/paths.reducer';
import { isDevServerTask } from '../reducers/tasks.reducer';
Expand All @@ -25,11 +26,17 @@ import {
getBaseProjectEnvironment,
PACKAGE_MANAGER_CMD,
} from '../services/platform.service';
import { processLogger } from '../services/process-logger.service';

import type { Action } from 'redux';
import type { Saga } from 'redux-saga';
import type { Task, ProjectType } from '../types';

// Mapping type for config template variables '$port'
export type VariableMap = {
$port: string,
};

const chalk = new chalkRaw.constructor({ level: 3 });

export function* launchDevServer({ task }: Action): Saga<void> {
Expand Down Expand Up @@ -57,6 +64,8 @@ export function* launchDevServer({ task }: Action): Saga<void> {
}
);

processLogger(child, 'DEVSERVER');

// Now that we have a port/processId for the server, attach it to
// the task. The port is used for opening the app, the pid is used
// to kill the process
Expand Down Expand Up @@ -181,6 +190,8 @@ export function* taskRun({ task }: Action): Saga<void> {
}
);

processLogger(child, 'TASK');

// TODO: Does the renderer process still need to know about the child
// processId?
yield put(attachTaskMetadata(task, child.pid));
Expand Down Expand Up @@ -249,6 +260,8 @@ export function* taskRun({ task }: Action): Saga<void> {
}
);

processLogger(installProcess, 'EJECT_INSTALL');

// `waitForChildProcessToComplete` waits for proper exit before moving on
// otherwise the next tasks (UI related) run too early before `yarn install`
// is finished
Expand Down Expand Up @@ -349,27 +362,61 @@ const createStdioChannel = (
});
};

export const substituteConfigVariables = (
AWolf81 marked this conversation as resolved.
Show resolved Hide resolved
configObject: any,
variableMap: VariableMap
) => {
// e.g. $port inside args will be replaced with variable reference from variabeMap obj. {$port: port}
return Object.keys(configObject).reduce(
(config, key) => {
if (config[key] instanceof Array) {
// replace $port inside args array
config[key] = config[key].map(arg => variableMap[arg] || arg);
} else {
// check config[key] e.g. is {env: { PORT: '$port'} }
if (config[key] instanceof Object) {
// config[key] = {PORT: '$port'}, key = 'env'
config[key] = Object.keys(config[key]).reduce(
(newObj, nestedKey) => {
// use replacement value if available
newObj[nestedKey] =
variableMap[newObj[nestedKey]] || newObj[nestedKey];
return newObj;
},
{ ...config[key] }
);
}
}
// todo: add top level substiution - not used yet but maybe needed later e.g. { env: $port } won't be replaced.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo (substitution)

// Bad example but just to have it as reminder.
return config;
},
{ ...configObject }
);
};

export const getDevServerCommand = (
task: Task,
projectType: ProjectType,
port: string
) => {
switch (projectType) {
case 'create-react-app':
return {
args: ['run', task.name],
env: {
PORT: port,
},
};
case 'gatsby':
return {
args: ['run', task.name, '-p', port],
env: {},
};
default:
throw new Error('Unrecognized project type: ' + projectType);
const config = projectConfigs[projectType];

if (!config) {
throw new Error('Unrecognized project type: ' + projectType);
}

// Substitution is needed as we'd like to have $port as args or in env
// we can use it in either position and it will be subsituted with the port value here
const devServer = substituteConfigVariables(config.devServer, {
// pass every value that is needed in the commands here
$port: port,
});

return {
args: devServer.args,
env: devServer.env || {},
};
};

export const stripUnusableControlCharacters = (text: string) =>
Expand Down
23 changes: 13 additions & 10 deletions src/services/create-project.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import * as childProcess from 'child_process';
import * as fs from 'fs';
import * as path from 'path';
import * as uuid from 'uuid/v1';
import projectConfigs from '../config/project-types';
import { processLogger } from './process-logger.service';

import { COLORS } from '../constants';

Expand Down Expand Up @@ -74,6 +76,8 @@ export default (

const process = childProcess.spawn(instruction, args);

processLogger(process, 'CREATE_PROJECT');

process.stdout.on('data', onStatusUpdate);
process.stderr.on('data', onError);

Expand Down Expand Up @@ -107,9 +111,11 @@ export default (
// Gatsby specific fix - the command 'npx gatsby new ...' always sets the
// name key in package.json to `gatsby-starter-default`. Overwrite it so
// project is named correctly.
if (projectType === 'gatsby') {
packageJson.name = projectDirectoryName;
}
// if (projectType === 'gatsby') {
// packageJson.name = projectDirectoryName;
// }
// Todo: Check if always setting the name is a problem --> we don't want project-type specific stuff here
packageJson.name = projectDirectoryName;

const prettyPrintedPackageJson = JSON.stringify(packageJson, null, 2);

Expand Down Expand Up @@ -164,12 +170,9 @@ export const getBuildInstructions = (
// 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');
switch (projectType) {
case 'create-react-app':
return [command, 'create-react-app', projectPath];
case 'gatsby':
return [command, 'gatsby', 'new', projectPath];
default:
throw new Error('Unrecognized project type: ' + projectType);
if (!projectConfigs.hasOwnProperty(projectType)) {
throw new Error('Unrecognized project type: ' + projectType);
}

return [command, ...projectConfigs[projectType].create.args(projectPath)];
};
3 changes: 2 additions & 1 deletion src/services/dependencies.service.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// @flow
import { PACKAGE_MANAGER_CMD } from './platform.service';
import { processLogger } from './process-logger.service';
import * as childProcess from 'child_process';

import type { QueuedDependency } from '../types';
Expand All @@ -24,7 +25,7 @@ const spawnProcess = (
'exit',
code => (code ? reject(output.stderr) : resolve(output.stdout))
);
// logger(child) // service will be used here later
processLogger(child, 'DEPENDENCY');
});

export const getDependencyInstallationCommand = (
Expand Down
Loading