4. Add react app bridge and retrieve and log session tokens #13
Conversation
MuhammadFarag
commented
May 6, 2021
MuhammadFarag
commented
- Enable React
- Add Appbridge
- Retrieve and log session token
README.md
Outdated
| - SCOPES | ||
| - HOST_NAME |
There was a problem hiding this comment.
I've had comments before that we should prefix all of our env vars, so this would look something like:
| - SCOPES | |
| - HOST_NAME | |
| - SHOPIFY_APP_SCOPES | |
| - SHOPIFY_APP_HOST |
Is that doable?
There was a problem hiding this comment.
It should be 👍
package.json
Outdated
| "@shopify/app-bridge": "^1.30.0", | ||
| "@shopify/app-bridge-utils": "^1.30.0", |
There was a problem hiding this comment.
I mentioned this in another PR, but we should support App Bridge 2.0 from the get-go here - that's surely going to trip partners up later if we don't.
resources/js/react/App.jsx
Outdated
| import createApp from "@shopify/app-bridge"; | ||
| import { getSessionToken } from "@shopify/app-bridge-utils" | ||
|
|
||
| function App({shop, apiKey}) { |
There was a problem hiding this comment.
I'd suggest we copy the _app.js file from https://github.com/Shopify/shopify-app-node/blob/master/pages/_app.js which has a lot of things we'll end up needing to handle here (like authenticatedFetch and the custom X-Shopify-API-Request-Failure-Reauthorize-Url which we'll also need to add here (as part of the middleware to check for a valid session).
But we should definitely add a ClientRouter to it since that question has come up repeatedly in that repo.
There was a problem hiding this comment.
authenticated fetch is added in the following PR #14. I am just trying to create a smaller easier to review PRs. I added a todo for X-Shopify-API-Request-Failure-Reauthorize-Url on that PR as well.
Updated this note https://github.com/orgs/Shopify/projects/1185#card-59723312, so we don't forget about ClientRouter
There was a problem hiding this comment.
Sounds great, I think the redirect header won't come into play until we add the middleware for checking for a current session though! We can definitely move ahead with what we have here!
MuhammadFarag
force-pushed
the
unprotected-root-route
branch
from
063de38
to
4e9e29d
Compare
tests/Feature/CallbackTest.php
Outdated
| Context::$SESSION_STORAGE->storeSession($this->session); | ||
|
|
||
|
|
||
| $matcher = new HttpRequestMatcher( |
There was a problem hiding this comment.
We can probably trim this test down as we did the other one!
MuhammadFarag
force-pushed
the
add-react-app-bridge
branch
from
bc5776e
to
62adb6b
Compare
