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

RCTModalHostView is tied to tvOS / RCTTVRemoteHandler.h file #17027

Closed
ptomasroos opened this issue Nov 30, 2017 · 8 comments
Closed

RCTModalHostView is tied to tvOS / RCTTVRemoteHandler.h file #17027

ptomasroos opened this issue Nov 30, 2017 · 8 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@ptomasroos
Copy link
Contributor

ptomasroos commented Nov 30, 2017

Is this a bug report?

Yes

We're using cocoapods are not able to build at the latest available hash in the repo 71b498b

If we add tvOS to out podspec (which makes no sense since we only do iOS development, everything is able to compile). So if this is the intended behaviour (which I dont think) I would suggest we add RCTTVRemoteHandler.h into the Core subspec.

But since I dont believe this intended we can see that all TV files are excluded in the Core podspec
https://github.com/facebook/react-native/blob/master/React.podspec#L55

Have you read the Contributing Guidelines?

Yes

Environment

➜  mobile git:(master) ✗ react-native info
module.js:487
    throw err;
    ^

Error: Cannot find module 'metro-bundler/src/lib/formatBanner'
    at Function.Module._resolveFilename (module.js:485:15)
    at Function.Module._load (module.js:437:25)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (/Users/tomas/dev/app/mobile/node_modules/react-native/local-cli/server/checkNodeVersion.js:12:20)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)

Very likely because we're running against master

Steps to Reproduce

1 Run latest master with cocoapods

Our podfile

  # To use CocoaPods with React Native, you need to add this specific Yoga spec as well
  pod 'yoga', :path => react_native_path + '/ReactCommon/yoga'

  # Third party deps
  pod 'DoubleConversion', :podspec => react_native_path + '/third-party-podspecs/DoubleConversion.podspec'
  pod 'GLog', :podspec => react_native_path + '/third-party-podspecs/GLog.podspec'
  pod 'Folly', :podspec => react_native_path + '/third-party-podspecs/Folly.podspec'

  pod 'React', subspecs: [
    'Core',
    'CxxBridge',
    'ART',
    'RCTAnimation',
    'RCTPushNotification',
    'RCTActionSheet',
    'RCTGeolocation',
    'RCTImage',
    'RCTLinkingIOS',
    'RCTNetwork',
    'RCTSettings',
    'RCTText',
    'RCTVibration',
    'RCTWebSocket',
    'DevSupport'
  ], path: '../node_modules/react-native'
  1. This will fail that RCTTVRemoteHandler.hs file is not found

  2. add 'tvOs'

  pod 'React', subspecs: [
    'Core',
    'CxxBridge',
    'ART',
    'tvOS',
    'RCTAnimation',
    'RCTPushNotification',
    'RCTActionSheet',
    'RCTGeolocation',
    'RCTImage',
    'RCTLinkingIOS',
    'RCTNetwork',
    'RCTSettings',
    'RCTText',
    'RCTVibration',
    'RCTWebSocket',
    'DevSupport'
  ], path: '../node_modules/react-native'

And it will find it

Expected Behavior

I should be able to compile RCTModalHostView without the dependency of tvOS's header files

Actual Behavior

When compiling the builds failes

screen shot 2017-11-30 at 15 11 07

@mgriepentrog
Copy link
Contributor

@dlowder-salesforce Would wrapping this import in #if TARGET_OS_TV solve this issue?

@douglowder
Copy link
Contributor

@mgriepentrog yes that sounds like the right fix.

douglowder added a commit to douglowder/react-native that referenced this issue Jan 9, 2018
@douglowder
Copy link
Contributor

@mgriepentrog I just submitted a PR to fix this.

@fungilation
Copy link

fungilation commented Jan 9, 2018

I confirm adding 'tvOS' as a React pod subspec works around this Xcode build blocker.

@grabbou, cherry pick for 0.52 hotfix?

facebook-github-bot pushed a commit that referenced this issue Jan 9, 2018
Summary:
Fix issue #17027 (`RCTModalHostView` has a tvOS dependency that was not wrapped in `TARGET_OS_TV`)

Existing test automation should pass.

[GENERAL] [BUGFIX] [tvOS] Fix cocoapods compile issue in RCTModalHostView
Closes #17502

Differential Revision: D6688166

Pulled By: hramos

fbshipit-source-id: 38297f439f75a8303f59f83b92e004c6c73d9bf6
@grabbou
Copy link
Contributor

grabbou commented Jan 10, 2018

I'll cherry-pick it and release with next chunk of commits.

grabbou pushed a commit that referenced this issue Jan 10, 2018
Summary:
Fix issue #17027 (`RCTModalHostView` has a tvOS dependency that was not wrapped in `TARGET_OS_TV`)

Existing test automation should pass.

[GENERAL] [BUGFIX] [tvOS] Fix cocoapods compile issue in RCTModalHostView
Closes #17502

Differential Revision: D6688166

Pulled By: hramos

fbshipit-source-id: 38297f439f75a8303f59f83b92e004c6c73d9bf6
mathiasbynens pushed a commit to mathiasbynens/react-native that referenced this issue Jan 13, 2018
Summary:
Fix issue facebook#17027 (`RCTModalHostView` has a tvOS dependency that was not wrapped in `TARGET_OS_TV`)

Existing test automation should pass.

[GENERAL] [BUGFIX] [tvOS] Fix cocoapods compile issue in RCTModalHostView
Closes facebook#17502

Differential Revision: D6688166

Pulled By: hramos

fbshipit-source-id: 38297f439f75a8303f59f83b92e004c6c73d9bf6
@yar-malik
Copy link

adding 'tvOS' fixed this bug for me. Thanks a lot. Already wasted half a day

@douglowder
Copy link
Contributor

@hramos we can close this

@ptomasroos
Copy link
Contributor Author

ptomasroos commented Feb 16, 2018

Closing @dlowder-salesforce 9954499

@facebook facebook locked as resolved and limited conversation to collaborators Feb 16, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

7 participants