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

[podspec] Fixes current CI failures and allows tree shaking of native dev support code. #12602

Closed

Conversation

alloy
Copy link
Contributor

@alloy alloy commented Feb 27, 2017

  • The dev support code moved into a DevSupport subspec, meaning that only if the subspec is specified in the user’s Podfile will the packager client, dev menu, etc be included. This is mainly done through checks for header availability.

    It also improves the weird situation where you had to specify the RCTWebSocket subspec if you wanted to be able to use the packager client during development.

  • I removed hardcoding the release version in the podspec on release, because the podspec still relies on package.json when evaluating, so there’s no real point in not also getting the version number from there. This should remove any requirement to perform maintenance of the OSS release script regarding the podspec.

The podspec still depends on the accompanying NPM package’s
package.json file, so there’s not really any point in hardcoding the version.
* This allows more tree shaking.
* Removes RCTWebSocket dependency from Core in development mode.
@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. GH Review: review-needed labels Feb 27, 2017
@@ -116,7 +119,7 @@ - (void)start
sourceCode = source;
dispatch_group_leave(initModulesAndLoadSource);
} onProgress:^(RCTLoadingProgress *progressData) {
#ifdef RCT_DEV
#if RCT_DEV && __has_include("RCTDevLoadingView.h")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This used to check if the macro is defined, which it always is, so I made it consistent with the rest of the code base (which is to check its value).

@@ -18,13 +18,9 @@
#import "JSCSamplingProfiler.h"
#import "RCTBridge+Private.h"
#import "RCTBridgeModule.h"
#import "RCTDevMenu.h"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This import wasn’t actually required, so I left it out.

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Feb 27, 2017
@ericvicenti
Copy link
Contributor

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

@ericvicenti has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@alloy alloy deleted the podspec-with-separate-dev-support branch February 27, 2017 22:52
GaborWnuk pushed a commit to GaborWnuk/react-native that referenced this pull request Feb 28, 2017
…rt code.

Summary:
* The dev support code moved into a `DevSupport` subspec, meaning that only if the subspec is specified in the user’s Podfile will the packager client, dev menu, etc be included. This is mainly done through checks for header availability.

  It also improves the weird situation where you had to specify the `RCTWebSocket` subspec if you wanted to be able to use the packager client during development.

* I removed hardcoding the release version in the podspec on release, because the podspec still relies on `package.json` when evaluating, so there’s no real point in not also getting the version number from there. This should remove any requirement to perform maintenance of the OSS release script regarding the podspec.
Closes facebook#12602

Differential Revision: D4621021

Pulled By: ericvicenti

fbshipit-source-id: 6c208371fc40ea607809a6ab05dd3714ed9980cf
dudeinthemirror pushed a commit to dudeinthemirror/react-native that referenced this pull request Mar 1, 2017
…rt code.

Summary:
* The dev support code moved into a `DevSupport` subspec, meaning that only if the subspec is specified in the user’s Podfile will the packager client, dev menu, etc be included. This is mainly done through checks for header availability.

  It also improves the weird situation where you had to specify the `RCTWebSocket` subspec if you wanted to be able to use the packager client during development.

* I removed hardcoding the release version in the podspec on release, because the podspec still relies on `package.json` when evaluating, so there’s no real point in not also getting the version number from there. This should remove any requirement to perform maintenance of the OSS release script regarding the podspec.
Closes facebook#12602

Differential Revision: D4621021

Pulled By: ericvicenti

fbshipit-source-id: 6c208371fc40ea607809a6ab05dd3714ed9980cf
dudeinthemirror pushed a commit to dudeinthemirror/react-native that referenced this pull request Mar 1, 2017
…rt code.

Summary:
* The dev support code moved into a `DevSupport` subspec, meaning that only if the subspec is specified in the user’s Podfile will the packager client, dev menu, etc be included. This is mainly done through checks for header availability.

  It also improves the weird situation where you had to specify the `RCTWebSocket` subspec if you wanted to be able to use the packager client during development.

* I removed hardcoding the release version in the podspec on release, because the podspec still relies on `package.json` when evaluating, so there’s no real point in not also getting the version number from there. This should remove any requirement to perform maintenance of the OSS release script regarding the podspec.
Closes facebook#12602

Differential Revision: D4621021

Pulled By: ericvicenti

fbshipit-source-id: 6c208371fc40ea607809a6ab05dd3714ed9980cf
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. Import Started This pull request has been imported. This does not imply the PR has been approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants