Skip to content

fix: Fix flow erros#79

Merged
thymikee merged 3 commits intoreact-native-community:masterfrom
Esemesek:fix-flow-errors
Jan 11, 2019
Merged

fix: Fix flow erros#79
thymikee merged 3 commits intoreact-native-community:masterfrom
Esemesek:fix-flow-errors

Conversation

@Esemesek
Copy link
Member

@Esemesek Esemesek commented Jan 10, 2019

Fixing flow errors. In some places, I removed @flow annotation because its module was never entirely typed, and adding types would require much more effort, than this simple fix.

EDIT: Also removed dependencies command since it exists already in metro.
PR: facebook/metro#335
Renaming cmd in metro: facebook/metro#339

@Esemesek Esemesek requested a review from grabbou January 10, 2019 19:03
const packageName = fs
.readFileSync(`${appFolder}/src/main/AndroidManifest.xml`, 'utf8')
// $FlowFixMe
.match(/package="(.+?)"/)[1];
Copy link
Member

Choose a reason for hiding this comment

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

Match result may be null so that's a valid bug. We should catch such scenario and throw an error that package is missing in AndroidManifest.xml. Can be done as a followup

const scriptsDir = path.resolve(__dirname, '..', '..', 'scripts');
const launchPackagerScript = path.resolve(scriptsDir, scriptFile);
const procConfig = { cwd: scriptsDir };
const procConfig: Object = { cwd: scriptsDir };
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed?

@thymikee
Copy link
Member

Merging to unbreak master, but the comments remain valid and I'd appreciate some discussion/followup :)

@thymikee thymikee merged commit a88c78f into react-native-community:master Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants