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

Open links from new app screen in computer's browser #24843

Closed

Conversation

lucasbento
Copy link
Contributor

Summary

This PR is related to #24760 and adds the openURLInBrowser functionality introduced on react-native-community/cli#383.

Changelog

[General] [Changed] - Open links from new app in computer's browser.

Test Plan

  1. Run npm i to install the new dep (bumped @react-native-community/cli to v2.0.0-alpha.19);
  2. Open and run RNTester/RNTesterPods.xcworkspace;
  3. Search for new in the list of the RNTester, tap the first item and click on any link, that link should open in your computer's default browser.

If any of these steps don't work please let me know and we will fix them.

cc @cpojer.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 14, 2019
@pull-bot
Copy link

pull-bot commented May 14, 2019

Warnings
⚠️ 🔒 package.json - Changes were made to package.json. This will require a manual import by a Facebook employee.

Generated by 🚫 dangerJS against 0001833

Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

Super good! I'll import this and land it. Thank you so so so much :)

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.

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

package.json Outdated Show resolved Hide resolved
Co-Authored-By: Michał Pierzchała <thymikee@gmail.com>
@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @lucasbento in aa926e3.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label May 14, 2019
@hramos
Copy link
Contributor

hramos commented May 14, 2019

It looks like test_android started failing with this PR. @lucasbento can you take a look?

#!/bin/bash -eo pipefail
node cli.js bundle --max-workers 2 --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js
Failed to construct transformer:  TypeError: Cannot read property 'update' of undefined
    at /root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:150:23
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:14:24)
    at _next (/root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:34:9)
    at /root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:39:7
    at new Promise (<anonymous>)
    at /root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:31:12
    at Function.load (/root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:170:7)
    at new Bundler (/root/react-native/node_modules/metro/src/Bundler.js:50:45)
    at new IncrementalBundler (/root/react-native/node_modules/metro/src/IncrementalBundler.js:109:21)
(node:4725) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'update' of undefined
    at /root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:150:23
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:14:24)
    at _next (/root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:34:9)
    at /root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:39:7
    at new Promise (<anonymous>)
    at /root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:31:12
    at Function.load (/root/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:170:7)
    at new Bundler (/root/react-native/node_modules/metro/src/Bundler.js:50:45)
    at new IncrementalBundler (/root/react-native/node_modules/metro/src/IncrementalBundler.js:109:21)
(node:4725) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:4725) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
error Cannot read property 'update' of undefined. Run CLI with --verbose flag for more details.
Exited with code 1

@hramos
Copy link
Contributor

hramos commented May 14, 2019

Here's how I verified the regression was introduced in the commit associated with this PR:

$ git reset 248a108abf206b7ae32208537f0b73a8192a4829 --hard
HEAD is now at 248a108abf fix scrollToLocation not consistent between platforms (#24734)
  • Confirm the Android JS bundle can be built:
$ yarn
Using globally installed version of Yarn
yarn install v1.12.1
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
success Already up-to-date.
✨  Done in 0.71s.
$ node cli.js bundle --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js --verbose
Loading dependency graph, done.
info Writing bundle output to:, ReactAndroid/src/androidTest/assets/AndroidTestBundle.js
info Done writing bundle output
warn Assets destination folder is not set, skipping...
  • Check out the commit where this PR was merged:
$ git reset aa926e349b1656b02b8c1a2048cc56b25f9567c1 --hard
HEAD is now at aa926e349b Open links from new app screen in computer's browser (#24843)
  • Verify bundle can be built (fails):
$ yarn
Using globally installed version of Yarn
yarn install v1.12.1
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/5] 🔍  Validating package.json...
[2/5] 🔍  Resolving packages...
[3/5] 🚚  Fetching packages...
[4/5] 🔗  Linking dependencies...
warning " > @react-native-community/cli@2.0.0-alpha.19" has unmet peer dependency "react-native@^0.60.0".
warning "fbjs-scripts > babel-preset-fbjs > @babel/plugin-check-constants@7.0.0-beta.38" has incorrect peer dependency "@babel/core@7.0.0-beta.38".
warning " > eslint-config-fbjs@2.1.0" has incorrect peer dependency "babel-eslint@^7.2.3 || ^8.0.0 || ^9.0.0".
warning " > eslint-config-fbjs@2.1.0" has incorrect peer dependency "eslint-plugin-relay@~0.0.8".
[5/5] 📃  Building fresh packages...
✨  Done in 3.24s.
$ node cli.js bundle --platform android --dev true --entry-file ReactAndroid/src/androidTest/js/TestBundle.js --bundle-output ReactAndroid/src/androidTest/assets/AndroidTestBundle.js --verbose
Failed to construct transformer:  TypeError: Cannot read property 'update' of undefined
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:150:23
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:14:24)
    at _next (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:34:9)
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:39:7
    at new Promise (<anonymous>)
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:31:12
    at Function.load (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:170:7)
    at new Bundler (/Users/hramos/git/react-native/node_modules/metro/src/Bundler.js:50:45)
    at new IncrementalBundler (/Users/hramos/git/react-native/node_modules/metro/src/IncrementalBundler.js:109:21)
(node:77790) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'update' of undefined
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:150:23
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:14:24)
    at _next (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:34:9)
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:39:7
    at new Promise (<anonymous>)
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:31:12
    at Function.load (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:170:7)
    at new Bundler (/Users/hramos/git/react-native/node_modules/metro/src/Bundler.js:50:45)
    at new IncrementalBundler (/Users/hramos/git/react-native/node_modules/metro/src/IncrementalBundler.js:109:21)
(node:77790) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2)
(node:77790) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
error Cannot read property 'update' of undefined
debug TypeError: Cannot read property 'update' of undefined
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:150:23
    at Generator.next (<anonymous>)
    at asyncGeneratorStep (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:14:24)
    at _next (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:34:9)
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:39:7
    at new Promise (<anonymous>)
    at /Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:31:12
    at Function.load (/Users/hramos/git/react-native/node_modules/metro/src/node-haste/DependencyGraph.js:170:7)
    at new Bundler (/Users/hramos/git/react-native/node_modules/metro/src/Bundler.js:50:45)
    at new IncrementalBundler (/Users/hramos/git/react-native/node_modules/metro/src/IncrementalBundler.js:109:21)
$ 

@thymikee
Copy link
Contributor

We have a fix for that coming: react-native-community/cli#376 will prioritize it today

@lucasbento
Copy link
Contributor Author

Thanks for pointing this @hramos.

Do you need help with it @thymikee?

@lucasbento lucasbento deleted the feat/open-links-in-browser branch May 15, 2019 14:29
facebook-github-bot pushed a commit that referenced this pull request May 16, 2019
Summary:
PR #24843 broke Android tests because of a regression we introduced in terms of passing `reporter` argument to Metro config. This was fixed in latest `alpha.20` release of CLI

## Changelog

[General] [Fix] - update CLI to alpha.20 to fix Android tests
Pull Request resolved: #24869

Reviewed By: rickhanlonii

Differential Revision: D15355144

Pulled By: cpojer

fbshipit-source-id: faafd8098c708845264b7164557076bce45ea332
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
Summary:
PR facebook#24843 broke Android tests because of a regression we introduced in terms of passing `reporter` argument to Metro config. This was fixed in latest `alpha.20` release of CLI

## Changelog

[General] [Fix] - update CLI to alpha.20 to fix Android tests
Pull Request resolved: facebook#24869

Reviewed By: rickhanlonii

Differential Revision: D15355144

Pulled By: cpojer

fbshipit-source-id: faafd8098c708845264b7164557076bce45ea332
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.

None yet

7 participants