Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 55 additions & 5 deletions packages/react-scripts/scripts/utils/frontierInit.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,21 @@ async function promptForConfig() {
{
type: 'checkbox',
name: 'additionalFeatures',
message: 'What additional features does your app require',
message: 'What additional features does your app require (these are checkboxes)',
default: ['electric-flow', 'header-footer'],
Copy link

Choose a reason for hiding this comment

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

I'm not sure we should default electric-flow to on

Choose a reason for hiding this comment

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

I do not know if this is true, but I was under the assumption that what Tyler did with the HF templating expects EF to be true/on as well.

Tyler would have to chime in further though.

choices: [
{
name: 'Using a shared Polymer Component within your React App?',
value: 'polymer',
},
{
name: `Configure app for Electric Flow`,
value: 'electric-flow',
},
{
name: `Include hf (header/footer)`,
value: 'header-footer',
},
{
name: 'Using a shared Polymer Component within your React App?',
value: 'polymer',
},
],
},
];
Expand All @@ -48,6 +53,9 @@ function installFrontierDependencies(appPath, answers, useYarn, ownPath) {
if (additionalFeatures.includes('electric-flow')) {
configureEF(appPath, useYarn, ownPath);
}
if (additionalFeatures.includes('header-footer')) {
configureHF(appPath, useYarn, ownPath);
Copy link

Choose a reason for hiding this comment

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

not specific to hf, but I don't think we should allow yarn as an option

Choose a reason for hiding this comment

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

I'm fine with that I guess, but if a developer doesn't use the --use-npm option, then they'll get react, react-dom, and @fs/react-scripts installed with yarn, and then all of "our" dependencies installed with npm.

I don't know if anyone would ever use create-react-app that way though, they should just use the fr-cli.

Choose a reason for hiding this comment

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

I'll go ahead and make this change to remove the yarn option completely in our frontierInit code

}
injectPolymerCode(appPath);

const defaultModules = ['http-proxy-middleware@0.19.0', 'fs-webdev/exo'];
Expand Down Expand Up @@ -127,6 +135,40 @@ function configureEF(appPath, useYarn, ownPath) {

const templatePath = path.join(ownPath, 'template-ef');
fs.copySync(templatePath, appPath, { overwrite: true });

alterPackageJsonFile(appPath, appPackage => {
const packageJson = { ...appPackage };
const additionalScripts = {
"heroku-prebuild": "./heroku-prebuild.sh"
Copy link

Choose a reason for hiding this comment

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

can we put this file in a common location instead of copying into each app?

};
packageJson.scripts = sortScripts({ ...packageJson.scripts, ...additionalScripts });
return packageJson;
});
}

function configureHF(appPath, useYarn, ownPath) {

const templatePath = path.join(ownPath, 'template-hf');
fs.copySync(templatePath, appPath, { overwrite: true });

alterPackageJsonFile(appPath, appPackage => {
const packageJson = { ...appPackage };
const additionalScripts = {
"build:prod": "PUBLIC_URL=https://edge.fscdn.org/assets/ react-scripts build",
"heroku-postbuild": "npm run build:prod"
};
packageJson.scripts = sortScripts({ ...packageJson.scripts, ...additionalScripts });
packageJson.main = "./server.js";

return packageJson;
});

let modules = [
'github:fs-webdev/hf#cra',
'github:fs-webdev/snow#cra',
'github:fs-webdev/startup',
]
installModulesSync(modules, useYarn);
}

function cleanupFrontierCode(appPath) {}
Expand Down Expand Up @@ -154,3 +196,11 @@ function buildInstallCommandAndArgs(useYarn, saveDev = false) {
}
return { command, args };
}

function sortScripts(scripts){
const sortedScripts = {};
Object.keys(scripts).sort().forEach(function(key) {
sortedScripts[key] = scripts[key];
});
return sortedScripts;
}
1 change: 1 addition & 0 deletions packages/react-scripts/template-ef/.buildpacks
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
https://github.com/heroku/heroku-buildpack-nodejs.git
https://github.com/fs-webdev/heroku-buildpack-upload-static-assets-to-s3.git
9 changes: 9 additions & 0 deletions packages/react-scripts/template-ef/heroku-prebuild.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash

if [ "$NODE_ENV" == "production" ]; then
echo "-----> Building .npmrc and .netrc"

echo "//code.lds.org/artifactory/api/npm/npm-fhd/:_authToken=${NPM_TOKEN}" > .npmrc
echo "@fs:registry=https://code.lds.org/artifactory/api/npm/npm-fhd/" >> .npmrc
echo -e "machine github.com\n login $GITHUB_AUTH_TOKEN" > ~/.netrc
fi
1 change: 1 addition & 0 deletions packages/react-scripts/template-hf/Procfile
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
web: npx startup start -w ${WEB_CONCURRENCY:-4}
1 change: 1 addition & 0 deletions packages/react-scripts/template-hf/public/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<link rel="manifest" href="%PUBLIC_URL%/manifest.json" />
Copy link

Choose a reason for hiding this comment

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

how does this work with the existing CRA index.html?

28 changes: 28 additions & 0 deletions packages/react-scripts/template-hf/server.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
const setProxies = require('exo/proxy');
const snow = require('snow');
const setWebpackManifest = require('snow/lib/utils/webpackManifest.js');
const snowConfig = require('./snow.config.js');
const hf = require('hf');

/**
* Expose the app
*/
const app = module.exports = snow(__dirname, hf, snowConfig);

setWebpackManifest(app,'build');

if (app.get('env') === 'development') {
app.stack.front(function () {
setProxies(app);
});
}


app.stack.postRoute(function () {
app.get('/',(req,res)=>{
res.render('index', {
indexPath: '../build/_index.html',
// _layoutFile: './async_layout'
})
});
});
27 changes: 27 additions & 0 deletions packages/react-scripts/template-hf/snow.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
var urlLookup = {
'cas-public-api.cas.ident.service': process.env.BASE_URL + '/cas-public-api',
Copy link

Choose a reason for hiding this comment

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

these fallbacks use the old pattern and should be updated to the new pattern

'cis-public-api.cis.ident.service': process.env.BASE_URL + '/cis-public-api',
};

var serviceLocatorOptions = {
fallbackFunction: function (serviceName) {
if (urlLookup[serviceName]) {
return urlLookup[serviceName];
}
throw new Error(`${serviceName} was not found in binding registry or urlLookup`);
}
};

module.exports = {
experiments : [
{
// Author/Owner: Tyler Graf
Copy link

Choose a reason for hiding this comment

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

I believe we recommend you put author/owner in the description now

name: 'coolExperiment',
Copy link

Choose a reason for hiding this comment

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

this should follow our naming convention

description: 'The coolest experiment',
default: true
Copy link

Choose a reason for hiding this comment

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

I would probably do default: false since that is the more common scenario

}
],
proxyUser: true,
cacheEncryption: true,
Copy link

Choose a reason for hiding this comment

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

Not sure how I feel about proxyUser on by default. And we should discuss making cacheEncryption defult to true

serviceLocatorOptions
}
24 changes: 24 additions & 0 deletions packages/react-scripts/template-hf/src/setupProxy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
const setProxies = require('exo/proxy');
const hf = require('hf');
const snow = require('snow');
const path = require('path');
const waitForWebpack = require('snow/lib/utils/waitForWebpack.js');

const initiatedDirectory = process.env.INIT_CWD;
Copy link

Choose a reason for hiding this comment

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

who sets INIT_CWD?

const snowConfig = require(path.join(initiatedDirectory, 'snow.config.js'));

module.exports = app => {
setProxies(app);
waitForWebpack(app);

snowConfig.app = app;
snow(initiatedDirectory, hf, snowConfig);

app.get('/', (req, res, next) => {
res.render('index', {
indexPath: '../dist/_index.html',
// _layoutFile: './async_layout'
});
});

};
9 changes: 9 additions & 0 deletions packages/react-scripts/template-hf/views/index.ejs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<%
page.secondaryNav = [
{"href": appPath("/"), title: 'Stuff', "text": 'Stuff'},
];
page.showHelper = true;
Copy link

Choose a reason for hiding this comment

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

do we fell that every app should have helper and secondaryNav setup by default? what about several other page variables that they could enable?

%>
<h1>hi</h1>
Copy link

Choose a reason for hiding this comment

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

Hi to you to. But I don't think it's useful in the default template

<div id="root"></div>
<%- include(indexPath) %>
4 changes: 4 additions & 0 deletions packages/react-scripts/template/gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,12 @@
# production
/build

# development
/dist
Copy link

Choose a reason for hiding this comment

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

wait, we create a dist for dev and a build for prod?


# misc
.DS_Store
.env
.env.local
Copy link

Choose a reason for hiding this comment

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

do we have some sort of pattern for alternate .env files? Should we just do .env.*? Or does something generate files like this, and if so, why?

.env.development.local
.env.test.local
Expand Down