-
Notifications
You must be signed in to change notification settings - Fork 932
feat: filter out non-native deps from config output; fix missing hooks #436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,36 +2,90 @@ | |
| * @flow | ||
| */ | ||
| import path from 'path'; | ||
| import {mapValues} from 'lodash'; | ||
| import chalk from 'chalk'; | ||
|
|
||
| import {logger, inlineString} from '@react-native-community/cli-tools'; | ||
| import * as ios from '@react-native-community/cli-platform-ios'; | ||
| import * as android from '@react-native-community/cli-platform-android'; | ||
| import findDependencies from './findDependencies'; | ||
| import resolveReactNativePath from './resolveReactNativePath'; | ||
| import findAssets from './findAssets'; | ||
| import makeHook from './makeHook'; | ||
| import { | ||
| readConfigFromDisk, | ||
| readDependencyConfigFromDisk, | ||
| } from './readConfigFromDisk'; | ||
|
|
||
| import {type ConfigT} from 'types'; | ||
|
|
||
| import assign from '../assign'; | ||
| import merge from '../merge'; | ||
| import resolveNodeModuleDir from './resolveNodeModuleDir'; | ||
| /** | ||
| * Built-in platforms | ||
| */ | ||
| import * as ios from '@react-native-community/cli-platform-ios'; | ||
| import * as android from '@react-native-community/cli-platform-android'; | ||
| import {logger, inlineString} from '@react-native-community/cli-tools'; | ||
|
|
||
| function getDependencyConfig( | ||
| root, | ||
| dependencyName, | ||
| finalConfig, | ||
| config, | ||
| userConfig, | ||
| isPlatform, | ||
| ) { | ||
| return merge( | ||
| { | ||
| root, | ||
| name: dependencyName, | ||
| platforms: Object.keys(finalConfig.platforms).reduce( | ||
| (dependency, platform) => { | ||
| // Linking platforms is not supported | ||
| dependency[platform] = isPlatform | ||
| ? null | ||
| : finalConfig.platforms[platform].dependencyConfig( | ||
| root, | ||
| config.dependency.platforms[platform] || {}, | ||
| ); | ||
| return dependency; | ||
| }, | ||
| {}, | ||
| ), | ||
| assets: findAssets(root, config.dependency.assets), | ||
| hooks: config.dependency.hooks, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the only change in this moved block is this line – we pass the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, right!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess an alternative take would be to provide a special
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to keep the whole config serializable anyway, less things to remember
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I was suggesting to make They are going to be gone (most likely) in the future anyway, so not bothering to much with that one. |
||
| params: config.dependency.params, | ||
| }, | ||
| userConfig.dependencies[dependencyName] || {}, | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Loads CLI configuration | ||
| */ | ||
| function loadConfig(projectRoot: string = process.cwd()): ConfigT { | ||
| const userConfig = readConfigFromDisk(projectRoot); | ||
|
|
||
| const initialConfig: ConfigT = { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this block was moved without modifications |
||
| root: projectRoot, | ||
| get reactNativePath() { | ||
| return userConfig.reactNativePath | ||
| ? path.resolve(projectRoot, userConfig.reactNativePath) | ||
| : resolveReactNativePath(projectRoot); | ||
| }, | ||
| dependencies: {}, | ||
| commands: userConfig.commands, | ||
| get assets() { | ||
| return findAssets(projectRoot, userConfig.assets); | ||
| }, | ||
| platforms: userConfig.platforms, | ||
| haste: { | ||
| providesModuleNodeModules: [], | ||
| platforms: Object.keys(userConfig.platforms), | ||
| }, | ||
| get project() { | ||
| const project = {}; | ||
| for (const platform in finalConfig.platforms) { | ||
| project[platform] = finalConfig.platforms[platform].projectConfig( | ||
| projectRoot, | ||
| userConfig.project[platform] || {}, | ||
| ); | ||
| } | ||
| return project; | ||
| }, | ||
| }; | ||
|
|
||
| const finalConfig = findDependencies(projectRoot).reduce( | ||
| (acc: ConfigT, dependencyName) => { | ||
| let root; | ||
|
|
@@ -53,9 +107,8 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { | |
| } | ||
|
|
||
| /** | ||
| * This workaround is necessary for development only before | ||
| * first 0.60.0-rc.0 gets released and we can switch to it | ||
| * while testing. | ||
| * @todo: remove this code once `react-native` is published with | ||
| * `platforms` and `commands` inside `react-native.config.js`. | ||
| */ | ||
| if (dependencyName === 'react-native') { | ||
| if (Object.keys(config.platforms).length === 0) { | ||
|
|
@@ -72,39 +125,24 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { | |
| * Legacy `rnpm` config required `haste` to be defined. With new config, | ||
| * we do it automatically. | ||
| * | ||
| * Remove this once `rnpm` config is deprecated. | ||
| * @todo: Remove this once `rnpm` config is deprecated and all major RN libs are converted. | ||
| */ | ||
| const haste = config.haste || { | ||
| providesModuleNodeModules: isPlatform ? [dependencyName] : [], | ||
| platforms: Object.keys(config.platforms), | ||
| }; | ||
|
|
||
| return assign({}, acc, { | ||
| return (assign({}, acc, { | ||
| dependencies: assign({}, acc.dependencies, { | ||
| // $FlowExpectedError: Dynamic getters are not supported | ||
| get [dependencyName]() { | ||
| return merge( | ||
| { | ||
| root, | ||
| name: dependencyName, | ||
| platforms: Object.keys(finalConfig.platforms).reduce( | ||
| (dependency, platform) => { | ||
| // Linking platforms is not supported | ||
| dependency[platform] = isPlatform | ||
| ? null | ||
| : finalConfig.platforms[platform].dependencyConfig( | ||
| root, | ||
| config.dependency.platforms[platform] || {}, | ||
| ); | ||
| return dependency; | ||
| }, | ||
| {}, | ||
| ), | ||
| assets: findAssets(root, config.dependency.assets), | ||
| hooks: mapValues(config.dependency.hooks, makeHook), | ||
| params: config.dependency.params, | ||
| }, | ||
| userConfig.dependencies[dependencyName] || {}, | ||
| return getDependencyConfig( | ||
| root, | ||
| dependencyName, | ||
| finalConfig, | ||
| config, | ||
| userConfig, | ||
| isPlatform, | ||
| ); | ||
| }, | ||
| }), | ||
|
|
@@ -120,36 +158,9 @@ function loadConfig(projectRoot: string = process.cwd()): ConfigT { | |
| ], | ||
| platforms: [...acc.haste.platforms, ...haste.platforms], | ||
| }, | ||
| }); | ||
| }): ConfigT); | ||
| }, | ||
| ({ | ||
| root: projectRoot, | ||
| get reactNativePath() { | ||
| return userConfig.reactNativePath | ||
| ? path.resolve(projectRoot, userConfig.reactNativePath) | ||
| : resolveReactNativePath(projectRoot); | ||
| }, | ||
| dependencies: {}, | ||
| commands: userConfig.commands, | ||
| get assets() { | ||
| return findAssets(projectRoot, userConfig.assets); | ||
| }, | ||
| platforms: userConfig.platforms, | ||
| haste: { | ||
| providesModuleNodeModules: [], | ||
| platforms: Object.keys(userConfig.platforms), | ||
| }, | ||
| get project() { | ||
| const project = {}; | ||
| for (const platform in finalConfig.platforms) { | ||
| project[platform] = finalConfig.platforms[platform].projectConfig( | ||
| projectRoot, | ||
| userConfig.project[platform] || {}, | ||
| ); | ||
| } | ||
| return project; | ||
| }, | ||
| }: ConfigT), | ||
| initialConfig, | ||
| ); | ||
|
|
||
| return finalConfig; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good idea to do it here - we still keep the lazy behaviour in other places
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that was the intention. Should I leave a comment to make it obvious?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine like this :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to approve it then? 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Be my guest