Skip to content

Conversation

@troZee
Copy link
Member

@troZee troZee commented Sep 11, 2019

Part of #683 (comment)

Current progress

  • findAssets-test
  • findDependencies-test
  • index-test
  • errors
  • findAssets
  • findDependencies
  • index
  • readConfigFromDisk
  • resolveNodeModuleDir
  • resolveReactNativePath
  • schema

Problem description:

File readConfigFromDisk.js contains methods, which use cosmiconfig dependency or just convert package.json file. I have a problem to type this readers correctly and map returned values into UserConfig or UserDependencyConfig. All methods from readConfigFromDisk file should return UserConfig or UserDependencyConfig(this type should be created based on schema validator).

@troZee troZee mentioned this pull request Sep 11, 2019
31 tasks
@troZee troZee force-pushed the config-ts-migration branch 3 times, most recently from cd9e2ad to 6e0766b Compare September 11, 2019 14:12
@nickytonline
Copy link

nickytonline commented Sep 12, 2019

Problem description:

File readConfigFromDisk.js contains methods, which use cosmiconfig dependency or just convert package.json file. I have a problem to type this readers correctly and map returned values into UserConfig or UserDependencyConfig. All methods from readConfigFromDisk file should return UserConfig or UserDependencyConfig(this type should be created based on schema validator).

It's not clear to me what you're having a hard time typing. Is it migrating the type from Flow to TypeScript that is the issue for you?

I created a TypeScript playground for getting started with the ConfigT and UserConfigT conversion from Flow.

I also came across this utility tonight which you may find helpful, care of Khan Academy, https://flow-to-ts.netlify.com.

@troZee
Copy link
Member Author

troZee commented Sep 12, 2019

There was a problem with UserConfig. This type returned never instead of object. It should be like you provided export type UserConfig = Omit<Config, 'root' | 'haste' | 'reactNativePath'>. That was really helpful @nickytonline, thank you.

@thymikee thymikee changed the title chore: wip: convert tools/config folder to ts chore: convert tools/config folder to TS Sep 16, 2019
@thymikee
Copy link
Member

@PTROCKI I've pushed a bunch of adjustments to unblock you. This looks mostly good now, but I'd like you and @Esemesek to check it once again.

@troZee
Copy link
Member Author

troZee commented Sep 19, 2019

Tested below commands:

node ./cli/packages/cli/build/index.js init AwesomeCLI 
cd AwesomeCLI
yarn add -D @react-native-community/netinfo
cd ios && pod install && cd ..
node ../cli/packages/cli/build/index.js run-ios
node ../cli/packages/cli/build/index.js run-android
node ../cli/packages/cli/build/index.js config  

and it works for both platforms. In this case netinfo was added to project correctly.

Copy link
Member

@Esemesek Esemesek left a comment

Choose a reason for hiding this comment

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

Apart from 2 comments I left PR is LGTMT.

@thymikee thymikee merged commit 221dec6 into react-native-community:master Sep 26, 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.

4 participants