Skip to content
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

Enable platforms to configure CLI commands #17745

Closed
wants to merge 1 commit into from
Closed

Enable platforms to configure CLI commands #17745

wants to merge 1 commit into from

Conversation

rozele
Copy link
Contributor

@rozele rozele commented Jan 25, 2018

This change adds hooks via the package.json for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in link and unlink commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.

Motivation

We need to be able to configure the CLI commands for plugin platforms (e.g., react-native-windows) in their own repositories.

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

  • All jest tests, including new tests, pass.
  • link and unlink commands are successful on iOS and Android.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

microsoft/react-native-windows#1601

Release Notes

[CLI][FEATURE][local-cli/core/index.js] - Allow platform plugins to contribute to the RNConfig.
[CLI][FEATURE][local-cli/link/link.js] - Allow platform plugins to participate in the link command.
[CLI][FEATURE][local-cli/link/unlink.js] - Allow platform plugins to participate in the unlink command.

@rozele rozele requested review from grabbou and Kureev as code owners January 25, 2018 19:30
@pull-bot
Copy link

pull-bot commented Jan 25, 2018

Warnings
⚠️

❗ Big PR - This PR is extremely unlikely to get reviewed because it touches 733 lines.

@facebook-github-bot label Android

@facebook-github-bot large-pr

Attention: @grabbou, @Kureev

Generated by 🚫 dangerJS

dependency[platform],
project[platform]
);

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

expect(findPlugins([ROOT])).toHaveProperty('commands');
expect(findPlugins([ROOT])).toHaveProperty('platforms');
expect(findPlugins([ROOT]).commands).toHaveLength(2);
expect(findPlugins([ROOT]).platforms).toHaveLength(0);

Choose a reason for hiding this comment

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

prettier/prettier: Delete ····

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. cla signed labels Jan 25, 2018

log.info(`Windows module ${packageName} has been successfully unlinked`);
log.info(`Unlinking ${packageName} ${platform} dependency`);

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

}

return pollParams(dependency.config.params).then(params => {
log.info(`Linking ${dependency.name} ${platform} dependency`);

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

params,
project[platform]
);

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

@@ -55,10 +58,20 @@ const attachPackage = (command, pkg) => Array.isArray(command)
? command.map(cmd => attachPackage(cmd, pkg))
: { ...command, pkg };

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

'../android/isInstalled.js',
sinon.stub().returns(true)
);

Choose a reason for hiding this comment

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

jest/valid-expect: No assertion was called on expect().


log.info(`Windows module ${packageName} has been successfully unlinked`);
log.info(`Unlinking ${packageName} ${platform} dependency`);

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

dependency[platform],
project[platform]
);

Choose a reason for hiding this comment

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

no-trailing-spaces: Trailing spaces not allowed.

@grabbou
Copy link
Contributor

grabbou commented Jan 25, 2018

This is great! Looking into the code right now, but I think the ability to extend built-in commands is another big step towards decoupling CLI from being android / ios only.

Copy link
Contributor

@grabbou grabbou left a comment

Choose a reason for hiding this comment

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

Leaving some preliminary feedback. Will probably get more as I dive deeper into the codebase.

Please let me know what you think :)

const plugins = folders.map(findPluginInFolder);
return {
commands: uniq(flatten(plugins.map(p => flatten(p.commands)))),
platforms: uniq(flatten(plugins.map(p => flatten(p.platforms))))
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure I am getting that line - the commands flattening is to make a flat array of all commands (if plugin defines many, we handle that).

What's the purpose of it for platforms? I get that we want to get a list of all platforms defined throughout the plugins - but when do we use it later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if the question here is related to platforms and why it's mangled with commands, or specifically why we flatten the platforms collection, so I'll address both.

platforms is used by local-cli/core/index.js and the link and unlink command to evaluate projectConfig(), dependencyConfig(), and linkConfig(). I would have created a separate module to extract the platform info, but I didn't want to impact CLI perf. If I created a separate module, I would have to read package.json for each potential plugin twice (instead of once as is done here).

In terms of why we flatten the collection of platforms - plugins could potentially define multiple platforms (e.g., uwp and win32, or ios and android), so I flatten the list so we have one array of all platforms used for configuration and in link.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@grabbou - I removed the inner flatten. The problem was that I had switched from concat to push.

return flatten(commands);
},

getPlatformConfig(): Object {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this can be just a function, so that no need to call this.... later to refer to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I'll fix.

Copy link
Contributor Author

@rozele rozele Jan 26, 2018

Choose a reason for hiding this comment

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

#WontFix - @grabbou, we need to expose the getPlatformConfig() on the RNConfig so that it can be used later by other commands (like link).

});
const linkDependencyPlatforms = (platforms, project, dependency) => {
const ignorePlatforms = ['android', 'ios'];
Object.keys(platforms || [])
Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, so the approach you have taken here is to explicitly do platform link. How about making it so that when plugin defines link command, it will be run like this:

originalLink().then(linkOfPlugin)

That way, we could not only allow react-native-windows to hook into link but potentially init and others. It would be also a bit less hardcoded then the following code.

Copy link
Contributor Author

@rozele rozele Jan 25, 2018

Choose a reason for hiding this comment

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

#WontFix - @grabbou For most commands there wouldn't be any problems with this. commander doesn't enforce one command to one key. E.g., I could easily define link in the react-native-windows project, and commander would just execute both the link from react-native and from react-native-windows. However - the semantics of link are as follows:

  1. Run prelink scripts
  2. Run link for all platforms
  3. Run postlink scripts

If I did originalLink().then(linkOfPlugin), we would probably need to breakdown prelink and postlink per platform as well (not the worst idea), e.g.:

  1. prelink for iOS
  2. link iOS
  3. postlink for iOS
  4. prelink for Android
    ...

That is a much more involved change that I didn't want to undertake.

Regarding init - init is a very difficult case because it's actually implemented partially by the react-native-cli global tool, and we currently do some heuristics to match the version of react-native-windows to the current installed version of react-native. Not saying its impossible, but I think init will require some other infrastructure. For other commands, there's no reason why we can't just define upgrade, eject, info, etc. in the plugim and just have it work as you propose. The best part is it doesn't require any code change because of the functionality built into commander!

log.info(`Windows module ${dependency.name} has been successfully linked`);
});
const linkDependencyPlatforms = (platforms, project, dependency) => {
const ignorePlatforms = ['android', 'ios'];
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why android and ios can't have the same code path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They can, I just wanted to keep the diff small.

Copy link
Contributor Author

@rozele rozele Jan 26, 2018

Choose a reason for hiding this comment

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

#WontFix - @grabbou There's currently some custom linking logic related to podfile for iOS. Let's do these changes in two phases:

  1. Remove windows and support platform extensions.
  2. Treat iOS and Android equivalent to any platform extension.

log.info(`Windows module ${dependency.name} has been successfully linked`);
});
const linkDependencyPlatforms = (platforms, project, dependency) => {
const ignorePlatforms = ['android', 'ios'];

Choose a reason for hiding this comment

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

element of Object.keys Expected object instead of empty array literal

@@ -37,11 +37,19 @@ const findPluginsInReactNativePackage = (pjson) => {
return path.join(pjson.name, pjson.rnpm.plugin);
};

const findPlatformsInPackage = (pjson) => {
if (!pjson.rnpm || !pjson.rnpm.plugin) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

plugin -> platform


try {
platforms = config.getPlatformConfig();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@grabbou - here is one example of where we need to call getPlatformConfig() on the RNConfig.

@grabbou
Copy link
Contributor

grabbou commented Jan 30, 2018

@Kureev please let me know what you think. I think this approach is temporarily okayish to me (although I am not a fan of custom platform-specific code in link and unlink itself).

@rozele
Copy link
Contributor Author

rozele commented Jan 31, 2018

@grabbou by custom platform-specific code, you're referring to the iOS and Android stuff, right? I really think the best way to move forward is incremental change (i.e., fix iOS and Android in a separate PR). It's safer / gives us more confidence we're not breaking iOS and Android and keeps the diff small. cc @Kureev

@rozele
Copy link
Contributor Author

rozele commented Jan 31, 2018

cc @hramos (I think you said you were interested in this PR).

@grabbou
Copy link
Contributor

grabbou commented Feb 1, 2018

@rozele true, I am just afraid that with no-one being interested in getting the codebase into its intended final shape, link will be left half-way done for a long time.

I have a plan on how to make this more "clean", but with you, being the only react-native-x I am aware (a platform), I am not sure if it's still a valid concern.

I guess we can merge it as is having another pair of the eyes to look at it.

@rozele rozele mentioned this pull request Feb 2, 2018
@rozele
Copy link
Contributor Author

rozele commented Feb 2, 2018

Hey @grabbou - this is no longer half-baked, can we merge without a second opinion?

@grabbou
Copy link
Contributor

grabbou commented Feb 2, 2018

Looks good to me - let's ship and I'll check before cherry-picking

@grabbou
Copy link
Contributor

grabbou commented Feb 2, 2018

@facebook-github-bot shipit

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Feb 2, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@grabbou is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@hramos
Copy link
Contributor

hramos commented Feb 8, 2018

This PR did not land as it could not be applied cleanly. Looks like this still needs to be rebased. Can you try again?

Copy link
Contributor

@Kureev Kureev left a comment

Choose a reason for hiding this comment

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

Wow! Great job, @rozele 🙌

@Kureev
Copy link
Contributor

Kureev commented Feb 8, 2018

Rebase (or let me know if I can do it for you) and ping me, I'll ship it 😉

This change adds hooks via the `package.json` for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in `link` and `unlink` commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.
@rozele
Copy link
Contributor Author

rozele commented Feb 8, 2018

Thanks @Kureev and @hramos - I just rebased.

@Kureev
Copy link
Contributor

Kureev commented Feb 8, 2018

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Something went wrong executing that command, @mkonicek could you take a look?

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Kureev is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@eastcoastcoder
Copy link

Any idea when this will land? Not having a functional link for UWP apps is a real downer.

@grabbou
Copy link
Contributor

grabbou commented Feb 12, 2018

@ethanx94 this is out as a 0.54.0-rc.0 right now - I'll be releasing patches in a backwards compatible manner tomorrow.

facebook-github-bot pushed a commit that referenced this pull request Feb 13, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes #17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
grabbou pushed a commit that referenced this pull request Feb 13, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes #17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
grabbou pushed a commit that referenced this pull request Feb 14, 2018
Summary:
This change adds hooks via the `package.json` for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in `link` and `unlink` commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

We need to be able to configure the CLI commands for plugin platforms (e.g., react-native-windows) in their own repositories.

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

- All jest tests, including new tests, pass.
- `link` and `unlink` commands are successful on iOS and Android.

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

microsoft/react-native-windows#1601

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/core/index.js] - Allow platform plugins to contribute to the RNConfig.
[CLI][FEATURE][local-cli/link/link.js] - Allow platform plugins to participate in the `link` command.
[CLI][FEATURE][local-cli/link/unlink.js] - Allow platform plugins to participate in the `unlink` command.
Closes #17745

Differential Revision: D6883558

Pulled By: hramos

fbshipit-source-id: ea32fe21cedd4cc02c5c0d48229f2cdb2ac8142b
grabbou pushed a commit that referenced this pull request Feb 14, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes #17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
grabbou pushed a commit that referenced this pull request Feb 14, 2018
Summary:
This change adds hooks via the `package.json` for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in `link` and `unlink` commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

We need to be able to configure the CLI commands for plugin platforms (e.g., react-native-windows) in their own repositories.

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

- All jest tests, including new tests, pass.
- `link` and `unlink` commands are successful on iOS and Android.

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

microsoft/react-native-windows#1601

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/core/index.js] - Allow platform plugins to contribute to the RNConfig.
[CLI][FEATURE][local-cli/link/link.js] - Allow platform plugins to participate in the `link` command.
[CLI][FEATURE][local-cli/link/unlink.js] - Allow platform plugins to participate in the `unlink` command.
Closes #17745

Differential Revision: D6883558

Pulled By: hramos

fbshipit-source-id: ea32fe21cedd4cc02c5c0d48229f2cdb2ac8142b
grabbou pushed a commit that referenced this pull request Feb 14, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes #17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
grabbou pushed a commit that referenced this pull request Feb 14, 2018
Summary:
This change adds hooks via the `package.json` for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in `link` and `unlink` commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

We need to be able to configure the CLI commands for plugin platforms (e.g., react-native-windows) in their own repositories.

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

- All jest tests, including new tests, pass.
- `link` and `unlink` commands are successful on iOS and Android.

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

microsoft/react-native-windows#1601

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/core/index.js] - Allow platform plugins to contribute to the RNConfig.
[CLI][FEATURE][local-cli/link/link.js] - Allow platform plugins to participate in the `link` command.
[CLI][FEATURE][local-cli/link/unlink.js] - Allow platform plugins to participate in the `unlink` command.
Closes #17745

Differential Revision: D6883558

Pulled By: hramos

fbshipit-source-id: ea32fe21cedd4cc02c5c0d48229f2cdb2ac8142b
grabbou pushed a commit that referenced this pull request Feb 14, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes #17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
Plo4ox pushed a commit to Plo4ox/react-native that referenced this pull request Feb 17, 2018
Summary:
This change adds hooks via the `package.json` for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in `link` and `unlink` commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

We need to be able to configure the CLI commands for plugin platforms (e.g., react-native-windows) in their own repositories.

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

- All jest tests, including new tests, pass.
- `link` and `unlink` commands are successful on iOS and Android.

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

microsoft/react-native-windows#1601

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/core/index.js] - Allow platform plugins to contribute to the RNConfig.
[CLI][FEATURE][local-cli/link/link.js] - Allow platform plugins to participate in the `link` command.
[CLI][FEATURE][local-cli/link/unlink.js] - Allow platform plugins to participate in the `unlink` command.
Closes facebook#17745

Differential Revision: D6883558

Pulled By: hramos

fbshipit-source-id: ea32fe21cedd4cc02c5c0d48229f2cdb2ac8142b
Plo4ox pushed a commit to Plo4ox/react-native that referenced this pull request Feb 17, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

facebook#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes facebook#17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
This change adds hooks via the `package.json` for a platform plugin to specify hooks for generating platform configurations. This change also adds hooks to allow platform plugins to participate in `link` and `unlink` commands. The goal is to move platform-specific code for platform plugins into their own repositories / node_modules.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

We need to be able to configure the CLI commands for plugin platforms (e.g., react-native-windows) in their own repositories.

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)

- All jest tests, including new tests, pass.
- `link` and `unlink` commands are successful on iOS and Android.

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/react-native-website, and link to your PR here.)

microsoft/react-native-windows#1601

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/core/index.js] - Allow platform plugins to contribute to the RNConfig.
[CLI][FEATURE][local-cli/link/link.js] - Allow platform plugins to participate in the `link` command.
[CLI][FEATURE][local-cli/link/unlink.js] - Allow platform plugins to participate in the `unlink` command.
Closes facebook/react-native#17745

Differential Revision: D6883558

Pulled By: hramos

fbshipit-source-id: ea32fe21cedd4cc02c5c0d48229f2cdb2ac8142b
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

facebook/react-native#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes facebook/react-native#17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
@hramos hramos added the Merged This PR has been merged. label Mar 8, 2019
@react-native-bot react-native-bot removed Import Started This pull request has been imported. This does not imply the PR has been approved. Import Failed labels Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants