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

[cli][start] Added interstitial middleware #16560

Merged
merged 11 commits into from Mar 14, 2022

Conversation

EvanBacon
Copy link
Contributor

Why

How

  • Rebuilt the interstitial middleware from Expo CLI so that it reuses logic from the other middleware in the dev server.

Test Plan

  • Unit tests.

@EvanBacon EvanBacon requested a review from lukmccall March 8, 2022 19:13
@EvanBacon EvanBacon marked this pull request as draft March 8, 2022 19:14
@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label Mar 8, 2022
Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Mar 8, 2022
// Production: This will resolve when installed in the project.
resolveFrom.silent(this.projectRoot, 'expo/static/loading-page/index.html') ??
// Development: This will resolve when testing locally.
require.resolve('../../../../../static/loading-page/index.html');
Copy link
Member

Choose a reason for hiding this comment

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

No need for require.resolve -- use path.resolve(__dirname, stuff). That said, path.join(require.resolve('expo'), 'static/loading-page/index.html') may work and be more robust.

packages/expo/static/loading-page/index.html Outdated Show resolved Hide resolved
packages/expo/static/loading-page/index.html Outdated Show resolved Hide resolved
EvanBacon and others added 3 commits March 14, 2022 15:18
Co-authored-by: James Ide <ide@users.noreply.github.com>
Update InterstitialPageMiddleware.ts

Update InterstitialPageMiddleware-test.ts
@EvanBacon EvanBacon force-pushed the @evanbacon/expo/cli/start/interstitial-middleware branch from 0a037b0 to 69796a5 Compare March 14, 2022 20:29
@EvanBacon EvanBacon marked this pull request as ready for review March 14, 2022 20:38
Copy link
Member

@ide ide left a comment

Choose a reason for hiding this comment

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

  • Change oversized Base64 icon for SVG
  • Replace require.resolve with path.resolve(__dirname) -- require() goes away with ESM

packages/expo/static/loading-page/index.html Outdated Show resolved Hide resolved
@EvanBacon
Copy link
Contributor Author

EvanBacon commented Mar 14, 2022

Fixed icon

Screen Shot 2022-03-14 at 5 53 50 PM

@EvanBacon EvanBacon merged commit 93158ce into main Mar 14, 2022
@EvanBacon EvanBacon deleted the @evanbacon/expo/cli/start/interstitial-middleware branch March 14, 2022 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: passed checks ExpoBot has nothing to complain about
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants