Skip to content

Commit

Permalink
fix: improve overall run-ios experience and get rid of CFBundleIdenti…
Browse files Browse the repository at this point in the history
…fier (#139)

Fixes #107 

Summary:
---------

Bugs fixed:
- "CFBundleIdentifier" printed every time "run-ios" fails, shadowing the real cause for the problem
- "run-ios" running a previous/cached version of an app (usually when there was an error during build step that was ignored)

Changes made:
- Decided to turn Flow and ESLint and apply all the changes as I am working on it. I know the diff might be strange at first sight, but if you look closely, it's just cosmetics to satisfy various rules
- I also extracted some blocks to separate utilities for better readability
- I also removed a no longer needed check for CRNA which is deprecated (and IMO, is not needed)
- Converted few functions to "async" form for better readability
- Started capturing the "stderr" of "xcodebuild" and started relying on its error code in order to avoid going to next step when it fails
- Added a new type of error to differentiate from other errors and also be able to capture additional data. Logic and styling subject to change, encapsulated inside Logger.

Test Plan:
----------

Here's how the error messages look before/now:

While try to build a project with a wrong "schema" :

Before:
<img width="882" alt="screenshot 2019-01-31 at 18 07 43" src="https://user-images.githubusercontent.com/2464966/52071369-200a4c00-2583-11e9-8700-28766daae122.png">

After:
<img width="880" alt="screenshot 2019-01-31 at 18 05 23" src="https://user-images.githubusercontent.com/2464966/52071244-d91c5680-2582-11e9-83e7-e9e549a570cc.png">

While building a project that has a typo in "AppDelegate.m":

Before:
<img width="885" alt="screenshot 2019-01-31 at 18 08 19" src="https://user-images.githubusercontent.com/2464966/52071411-344e4900-2583-11e9-8613-96ddf6c68006.png">

After:
<img width="886" alt="screenshot 2019-01-31 at 18 06 36" src="https://user-images.githubusercontent.com/2464966/52071300-f6e9bb80-2582-11e9-9c9f-3a4b67318119.png">

You can try it out by running `run-ios` command on a broken project (typo, wrong configuration) or by passing a non-existing `schema`
  • Loading branch information
grabbou authored and Esemesek committed Feb 1, 2019
1 parent b33d079 commit 6526bdd
Show file tree
Hide file tree
Showing 6 changed files with 241 additions and 175 deletions.
2 changes: 1 addition & 1 deletion .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ suppress_type=$FlowFixMeProps
suppress_type=$FlowFixMeState

suppress_comment=\\(.\\|\n\\)*\\$FlowFixMe\\($\\|[^(]\\|(\\(<VERSION>\\)? *\\(site=[a-z,_]*react_native\\(_ios\\)?_\\(oss\\|fb\\)[a-z,_]*\\)?)\\)
suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\((\\(<VERSION>\\)? *\\(site=[a-z,_]*react_native\\(_ios\\)?_\\(oss\\|fb\\)[a-z,_]*\\)?)\\)?:? #[0-9]+
suppress_comment=\\(.\\|\n\\)*\\$FlowIssue\\($\\|[^(]\\|(\\(<VERSION>\\)? *\\(site=[a-z,_]*react_native\\(_ios\\)?_\\(oss\\|fb\\)[a-z,_]*\\)?)\\)
suppress_comment=\\(.\\|\n\\)*\\$FlowFixedInNextDeploy
suppress_comment=\\(.\\|\n\\)*\\$FlowExpectedError

Expand Down
17 changes: 7 additions & 10 deletions packages/cli/src/cliEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@ commander.version(pkg.version);
const defaultOptParser = val => val;

const handleError = err => {
logger.error(err.message || err);
if (err.stack) {
logger.error(err.stack);
}
logger.error(err.message);
process.exit(1);
};

Expand Down Expand Up @@ -77,12 +74,12 @@ function printUnknownCommand(cmdName) {
logger.error(
[
cmdName
? ` Unrecognized command "${cmdName}"`
: " You didn't pass any command",
` Run ${chalk.white(
'react-native --help'
)} to see list of all available commands`,
].join('')
? `Unrecognized command "${cmdName}".`
: "You didn't pass any command.",
`Run ${chalk.white(
'"react-native --help"'
)} to see list of all available commands.`,
].join(' ')
);
}

Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/core/getPlatforms.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ module.exports = function getPlatforms(root: string): PlatformsT {
(acc, pathToPlatform) =>
Object.assign(
acc,
// $FlowFixMe non-literal require
require(path.join(root, 'node_modules', pathToPlatform))
),
{}
Expand Down
Loading

0 comments on commit 6526bdd

Please sign in to comment.