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] fix js inspector is broken when using web SSG #23197

Merged
merged 3 commits into from Jun 30, 2023

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Jun 29, 2023

Why

when web SSG is enabled, pressing j to open js inspector is broken:

Opening JavaScript inspector in the browser...
Web Bundling complete 4499ms
Web Bundling complete 4524ms
FetchError: invalid json response body at http://192.168.50.38:8081/json/list reason: Unexpected token < in JSON at position 0
FetchError: invalid json response body at http://192.168.50.38:8081/json/list reason: Unexpected token < in JSON at position 0

How

originally we have #21068 fix for the HistoryFallbackMiddleware. the new middleware for SSG unfortunately does not have the fix.
i feel like the root cause is coming from middleware disorder. digging the code in deep, i found we've chained the connect app. the middleware returned from Metro.createConnectMiddleware is actually a connect app, because rn-cli will set the enhanceMiddleware and it creates the connect app under the hood.

this pr tries to reuse the connect app. without the connect app chaining, those web middlewares will be placed after the inspector proxy middleware.

also removed the #21068 hack for HistoryFallbackMiddleware in this pr.

Code References

Test Plan

$ yarn create expo -t tabs@sdk-49 sdk49tabs
$ cd sdk49tabs
$ npx expo start
# press `j`

after the fix, i've checked:

  1. press j
  2. check http://localhost:8081/json/list returns an array
  3. check http://localhost:8081/helloweb returns a html content
  4. disable SSG (the "output": "static" in app.json), check http://localhost:8081/helloweb returns a html content

Checklist

@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Jun 29, 2023
@Kudo Kudo marked this pull request as ready for review June 29, 2023 16:10
@byCedric
Copy link
Member

Replaces #23192

Copy link
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

ok

@Kudo Kudo merged commit c799f27 into main Jun 30, 2023
5 checks passed
@Kudo Kudo deleted the @kudo/sdk49/fix-inspector-with-ssg branch June 30, 2023 08:12
Kudo added a commit that referenced this pull request Jun 30, 2023
# Why

when web SSG is enabled, pressing `j` to open js inspector is broken:
```
Opening JavaScript inspector in the browser...
Web Bundling complete 4499ms
Web Bundling complete 4524ms
FetchError: invalid json response body at http://192.168.50.38:8081/json/list reason: Unexpected token < in JSON at position 0
FetchError: invalid json response body at http://192.168.50.38:8081/json/list reason: Unexpected token < in JSON at position 0
```

# How

originally we have #21068 fix for the HistoryFallbackMiddleware. the new middleware for SSG unfortunately does not have the fix.
i feel like the root cause is coming from middleware disorder. digging the code in deep, i found we've chained the `connect` app. the `middleware` returned from `Metro.createConnectMiddleware` is actually a connect app, because rn-cli will set the `enhanceMiddleware` and it creates the `connect` app under the hood.

this pr tries to reuse the connect app. without the connect app chaining, those web middlewares will be placed after the inspector proxy middleware.

also removed the #21068 hack for HistoryFallbackMiddleware in this pr.

# Test Plan

```
$ yarn create expo -t tabs@sdk-49 sdk49tabs
$ cd sdk49tabs
$ npx expo start
# press `j`
```

after the fix, i've checked:
1. press `j`
2. check `http://localhost:8081/json/list` returns an array
3. check `http://localhost:8081/helloweb` returns a html content
4. disable SSG (the `"output": "static"` in **app.json**), check `http://localhost:8081/helloweb` returns a html content

(cherry picked from commit c799f27)
@brentvatne brentvatne added the published Changes from the PR have been published to npm label Jun 30, 2023
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 published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants