Crash the app if does not find index.html, index.js or favico.ico #653
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks! |
ghost
added
the
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
| @@ -34,7 +34,9 @@ function resolveApp(relativePath) { | |||
| // config after eject: we're in ./config/ | |||
| module.exports = { | |||
| appBuild: resolveApp('build'), | |||
| appFavico: resolveApp('src/favicon.ico'), | |||
There was a problem hiding this comment.
Favicon is not required anymore. We can remove it from the list of files to check.
| appHtml: resolveApp('index.html'), | ||
| appIndexJs: resolveApp('src/index.js'), |
There was a problem hiding this comment.
Let’s use this variable instead of path.join(paths.appSrc, 'index') in webpack.config.dev and .prod too.
| function checkRequiredFiles() { | ||
| var filesPathToCheck = [paths.appHtml, paths.appIndexJs, paths.appFavico]; | ||
| filesPathToCheck.forEach(function(filePath) { | ||
| fs.access(filePath, fs.constants.F_OK, function(err) { |
There was a problem hiding this comment.
This is asynchronous so you are going to have race conditions. The code below will execute before these callbacks.
Instead, we should do a synchronous access here.
| var filesPathToCheck = [paths.appHtml, paths.appIndexJs, paths.appFavico]; | ||
| filesPathToCheck.forEach(function(filePath) { | ||
| fs.access(filePath, fs.constants.F_OK, function(err) { | ||
| if(err) { |
There was a problem hiding this comment.
Minor nit: please put space after if
|
Cherry-picked via 4a0f39a. I rebased to tag you as the commit author because your local commit email doesn’t match your GitHub email. You need to change local git settings to use your GitHub email so that in the future, your userpic shows up and your contributions get credited in project history. Cheers! |
Hi,
I saw that issue(#96) needed help, it had previously a PR that have been closed few weeks back. I don't know if someone is still working this. So I tried to solve the issue
How can I improve my PR ?
Thanks !