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

Update iOS Simulator device command in React Native docs #32383

Conversation

andrewserong
Copy link
Contributor

Description

While working through the getting started with native mobile docs, I noticed that I couldn't get npm run native ios to launch the iOS simulator with a custom device specified. Despite specifying a device, it would be stuck launching the iPhone 11.

It looks as thought we might need to pass the --simulator option down a couple of levels through npm when running the command. If I add two sets of -- before the --simulator option then I can see in the terminal that the react-native command gets the appropriate option. Without this, the --simulator is passed to either npm, or the next level down, which also appears to be npm via npm run --prefix packages/react-native-editor.

CC: @gwwar as I know you've been looking at getting up and running with native mobile.

How has this been tested?

Follow the instructions in the document and attempt to launch the React Native environment while specifying a custom device. For me, here are the results:

npm run native ios --simulator="iPhone 8 Plus" # ❌ Doesn't pick up custom device
npm run native ios -- --simulator="iPhone 8 Plus" # ❌ Doesn't pick up custom device
npm run native ios -- -- --simulator="iPhone 8 Plus" # ✅ Launches with iPhone 8 Plus

A question for the reader: can you replicate the issue above where it only works with the extra sets of double dashes (--)?

Screenshots

Types of changes

Checklist:

  • My code is tested (tested the documentation update manually)
  • (N/A) My code follows the WordPress code style.
  • (N/A) My code follows the accessibility standards.
  • (N/A) I've tested my changes with keyboard and screen readers.
  • (N/A) My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • (N/A) I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a comment

Choose a reason for hiding this comment

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

Thanks @andrewserong for the PR 👍

While taking a look at this I found that npm allows some primitive passing of CLI args along to a script. For example --simulator becomes available as $npm_config_simulator.

I gave that a quick try updating the ios script. It worked well when specifying a simulator device however when not, it appears to change the default device loaded. Updating the ios script further to conditionally add the --simulator argument might start to get into some cross environment issues.

For the moment, I think updating the docs so that following them actually works is definitely an improvement.

@aaronrobertshaw aaronrobertshaw merged commit 4c8fc2d into WordPress:trunk Jun 2, 2021
@github-actions github-actions bot added this to the Gutenberg 10.8 milestone Jun 2, 2021
@andrewserong
Copy link
Contributor Author

Ah, nice! Thanks for looking into that and merging in this PR @aaronrobertshaw! 🙇

@andrewserong andrewserong deleted the update/react-native-ios-docs-command-for-using-other-ios-devices branch June 6, 2021 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants