[cli] point QR code to interstitial page when enabled #18838

esamelson · 2022-08-26T00:03:06Z

Why

third step of ENG-6120 -- ports most of expo/expo-cli@e901f5e to new CLI

How

Add user-agent parsing to deduce the platform in cases where it isn't provided (i.e. when the user has opened the interstitial page via QR code)
Allow platform to be optional in getLoadingUrl
Show a QR code for the interstitial page when it's enabled (but still print the native runtime URL)
Don't restrict iOS interstitial page URL to localhost

Test Plan

Added unit tests for all new functionality. Also tested manually to ensure parity with the old CLI:

EXPO_ENABLE_INTERSTITIAL_PAGE=1 npx expo start + scan QR code -> see interstitial page
both buttons on interstitial page work
EXPO_ENABLE_INTERSTITIAL_PAGE=1 npx expo start --dev-client + scan QR code -> opens in dev client
remove expo-dev-client from project, EXPO_ENABLE_INTERSTITIAL_PAGE=1 npx expo start + scan QR code -> opens in Expo Go

Checklist

Documentation is up to date to reflect these changes (eg: https://docs.expo.dev and README.md).
Conforms with the Documentation Writing Style Guide
This diff will work correctly for expo prebuild & EAS Build (eg: updated a module plugin).

linear · 2022-08-26T00:03:08Z

ENG-6120 Take user to interstitial when launching dev client project from versioned CLI

pressing i
pressing a
passing ios or android flag

EvanBacon · 2022-08-29T08:53:00Z

packages/@expo/cli/src/start/server/BundlerDevServer.ts

@@ -348,6 +348,13 @@ export abstract class BundlerDevServer {
      : this.getUrlCreator().constructUrl({ ...opts, scheme: 'exp' });
  }

+  public getQRCodeUrl(opts: Partial<CreateURLOptions> = {}) {


There's no such concept as a "QR Code" URL, all URLs can be represented as a QR code.

I've removed this and changed interactiveActions to call getInterstitialPageUrl directly.

EvanBacon · 2022-08-29T08:55:32Z

packages/@expo/cli/src/start/server/middleware/RuntimeRedirectMiddleware.ts

@@ -36,7 +37,7 @@ export class RuntimeRedirectMiddleware extends ExpoMiddleware {
  async handleRequestAsync(req: ServerRequest, res: ServerResponse): Promise<void> {
    const { query } = parse(req.url!, true);
    const isDevClient = query['choice'] === 'expo-dev-client';
-    const platform = parsePlatformHeader(req);
+    const platform = parsePlatformHeader(req) ?? resolvePlatformFromUserAgentHeader(req);


parsePlatformHeader is used across almost all middleware, why should the policy for platform detection change for just one use-case.

EvanBacon · 2022-08-29T08:56:52Z

packages/@expo/cli/src/start/server/middleware/__tests__/resolvePlatform-test.ts

+          url: 'http://localhost:3000',
+          headers: {
+            'user-agent':
+              'Mozilla/5.0 (iPhone; CPU iPhone OS 15_2 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.2 Mobile/15E148 Safari/604.1',


These user agent values need annotation about where they come from and how to update them in the future.

EvanBacon · 2022-08-29T08:57:02Z

packages/@expo/cli/src/start/server/middleware/__tests__/resolvePlatform-test.ts

+      resolvePlatformFromUserAgentHeader(
+        asRequest({
+          url: 'http://localhost:3000',
+          headers: {},


When would this happen?

EvanBacon · 2022-08-29T08:57:56Z

packages/@expo/cli/src/start/server/middleware/resolvePlatform.ts

@@ -21,6 +21,18 @@ export function parsePlatformHeader(req: ServerRequest): string | null {
  return (Array.isArray(platform) ? platform[0] : platform) ?? null;
 }

+/** Guess the platform from the user-agent header. */


Add more informative doc block.

packages/@expo/cli/src/start/server/middleware/resolvePlatform.ts

packages/@expo/cli/src/start/interface/interactiveActions.ts

EvanBacon · 2022-08-29T09:57:00Z

packages/@expo/cli/src/start/interface/interactiveActions.ts

+        printQRCode(qrCodeUrl);
+        Log.log(printItem(chalk`Metro waiting on {underline ${nativeRuntimeUrl}}`));


The QR code is just a convenience feature, it should match the actual URL that is displayed. Showing a URL and a conditionally different QR code is confusing.

I thought that showing a message saying Metro waiting on <interstitial-page-url> would be both confusing and inaccurate. I’ve updated this to print both URLs along with another message when the QR code points to the interstitial page.

EvanBacon · 2022-08-29T09:59:04Z

packages/@expo/cli/src/start/server/BundlerDevServer.ts

@@ -403,6 +410,8 @@ export abstract class BundlerDevServer {
  protected shouldUseInterstitialPage(): boolean {
    return (
      env.EXPO_ENABLE_INTERSTITIAL_PAGE &&
+      // if user passed --dev-client flag, skip interstitial page
+      !this.isDevClient &&


If the user passed in --dev-client then this function should never be called, openAsync should use the custom option.

Without this here, the QR code would now point to interstitial page when --dev-client is passed.

EvanBacon · 2022-08-29T10:04:17Z

packages/@expo/cli/src/start/server/middleware/InterstitialPageMiddleware.ts

@@ -75,7 +76,7 @@ export class InterstitialPageMiddleware extends ExpoMiddleware {
    res = disableResponseCache(res);
    res.setHeader('Content-Type', 'text/html');

-    const platform = parsePlatformHeader(req);
+    const platform = parsePlatformHeader(req) ?? resolvePlatformFromUserAgentHeader(req);


This method is attempting to implement our hosted server's specification with added legacy support for expo-cli which supported the exponent-platform header, is platform sniffing worked into the specification anywhere?

EvanBacon

check comments

esamelson · 2022-08-30T19:53:19Z

The reason for adding the user-agent detection is because if the interstitial page is opened via a QR code, we need to detect what platform they’re on so that we can open the correct deep link when they press one of the buttons on the page. (This was added to expo-cli in expo/expo-cli@e901f5e.) If you have a different suggestion for how to achieve this, let me know and I can try implementing that instead.

…ys localhost

…QR code

…for interactiveActions

esamelson requested a review from lukmccall August 26, 2022 00:03

esamelson requested review from EvanBacon and byCedric as code owners August 26, 2022 00:03

esamelson force-pushed the @eric/eng-6120-2 branch from 687a834 to f45fd6c Compare August 26, 2022 00:03

lukmccall approved these changes Aug 26, 2022

View reviewed changes

EvanBacon reviewed Aug 29, 2022

View reviewed changes

packages/@expo/cli/src/start/server/middleware/resolvePlatform.ts Show resolved Hide resolved

EvanBacon reviewed Aug 29, 2022

View reviewed changes

packages/@expo/cli/src/start/interface/interactiveActions.ts Outdated Show resolved Hide resolved

EvanBacon reviewed Aug 29, 2022

View reviewed changes

EvanBacon requested changes Aug 29, 2022

View reviewed changes

esamelson force-pushed the @eric/eng-6120-2 branch from 48a88a9 to 276240d Compare August 30, 2022 22:03

esamelson requested a review from EvanBacon August 30, 2022 22:51

esamelson added a commit that referenced this pull request Sep 20, 2022

move side effect out of shouldUseInterstitialPage for #18838

cf25a0d

esamelson added 5 commits September 20, 2022 11:33

[cli] add user-agent parsing for platform in interstitial page handler

920a277

[cli] make QR code point to interstitial page and make links not alwa…

4ff7633

…ys localhost

changelog

9f7fd7e

update method calls; show extra message underneath interstitial page …

e3ed34d

…QR code

add user-agent debug log and comments to tests

e7c6a4b

esamelson force-pushed the @eric/eng-6120-2 branch from 6112afd to e7c6a4b Compare September 20, 2022 08:35

esamelson and others added 3 commits September 20, 2022 11:51

add getInterstitialPageUrl method that was lost after rebase, needed …

3960039

…for interactiveActions

feat(cli): clean up logic

e315ed2

updated tests

463e7c4

esamelson and others added 2 commits September 22, 2022 11:44

fix lint

e5b0c21

[cli] try to detect if deep link was successful (#18839)

a4b0519

EvanBacon requested review from Simek, amandeepmittal and kbrandwijk as code owners September 26, 2022 17:07

EvanBacon merged commit 14e5da3 into @eric/cli-ensure-dc-installed Sep 26, 2022

EvanBacon deleted the @eric/eng-6120-2 branch September 26, 2022 17:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[cli] point QR code to interstitial page when enabled #18838

[cli] point QR code to interstitial page when enabled #18838

esamelson commented Aug 26, 2022

linear bot commented Aug 26, 2022

EvanBacon Aug 29, 2022

esamelson Aug 30, 2022

EvanBacon Aug 29, 2022

EvanBacon Aug 29, 2022

EvanBacon Aug 29, 2022

EvanBacon Aug 29, 2022

EvanBacon Aug 29, 2022

esamelson Aug 30, 2022

EvanBacon Aug 29, 2022

esamelson Aug 30, 2022

EvanBacon Aug 29, 2022

EvanBacon left a comment

esamelson commented Aug 30, 2022

		printQRCode(qrCodeUrl);
		Log.log(printItem(chalk`Metro waiting on {underline ${nativeRuntimeUrl}}`));

[cli] point QR code to interstitial page when enabled #18838

[cli] point QR code to interstitial page when enabled #18838

Conversation

esamelson commented Aug 26, 2022

Why

How

Test Plan

Checklist

linear bot commented Aug 26, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EvanBacon left a comment

Choose a reason for hiding this comment

esamelson commented Aug 30, 2022