Modify components to support ionic fork #192
Conversation
| const headers = {}; | ||
| if (nextDataEl?.textContent) { | ||
| const data = JSON.parse(nextDataEl.textContent); | ||
| headers["CSRF-Token"] = data.query.CSRF_TOKEN; |
There was a problem hiding this comment.
No longer require a CSRF token
| }; | ||
|
|
||
| export const withApollo = (Component: any, options?: WithApolloOptions) => | ||
| options?.useNext === false |
There was a problem hiding this comment.
Optionally disable nextjs processing for apollo, but enable it by default
| @@ -1,5 +1,20 @@ | |||
| import csrf from "csurf"; | |||
| import { Express } from "express"; | |||
| import url from "url"; | |||
|
|
|||
| const skipList = process.env.CSRF_SKIP_REFERERS | |||
There was a problem hiding this comment.
Allow us to pass a list of referrers like:
CSRF_SKIP_REFERERS=localhost:8100,mobile.mydomain.com
| { initialState, ctx }: InitApolloOptions<NormalizedCacheObject>, | ||
| withApolloOptions?: WithApolloOptions | ||
| ): ApolloClient<NormalizedCacheObject> => { | ||
| const ROOT_URL = process.env.ROOT_URL || withApolloOptions?.rootUrl; |
There was a problem hiding this comment.
Allow us to pass in a different rootUrl in the options to withApollo
|
|
||
| import { GraphileApolloLink } from "./GraphileApolloLink"; | ||
|
|
||
| interface WithApolloOptions { |
There was a problem hiding this comment.
withApollo can now be called as:
withApollo(App, {
useNext: false,
rootUrl: process.env.REACT_APP_ROOT_URL,
});
justinr1234
force-pushed
the
ionic
branch
2 times, most recently
from
42e01a7
to
5c07533
Compare
| @@ -4,3 +4,4 @@ node_modules | |||
| /data/schema.sql | |||
| /@app/graphql/index.* | |||
| /@app/client/.next | |||
| **/build | |||
There was a problem hiding this comment.
Ignore any build folders, such as the one ionic generates
justinr1234
force-pushed
the
ionic
branch
4 times, most recently
from
d60ab39
to
9caa939
Compare
|
As it stands, this PR contains a lot of changes; as discussed on Discord, lets break it up into a number of bite-sized PRs that will be easier to review and iterate so we can merge things a bit faster. Please make sure that each PR comes with an explanation why this change is valuable in general (not specifically limited to Ionic, e.g. you might show how the change is also necessary for Create React App). Here's some PR topics that would make sense from this: Formatting changes
I notice that sudo apt-get update
This fix should be in place for everyone, please send it as a standalone PR. withApollo.ts -> .tsx
A simple rename of this file could be included such that the diff when you later edit it will be much clearer. Actually; since this is just a file rename I'll do it now... Done. cors
This is going to be a controversial addition which will require quite a bit of back-and-forth (but I'm generally in favour of merging, when done right); lets get it as a clean PR on its own. lerna run setup
Raise this as a separate PR including a small mention in the README. If it passes tests I see no reason not to merge it. Session changes (allowed origins)
Please raise a PR for just this change with a good explanation of why it's needed, whether it's needed only for development, and any security concerns/caveats there may be with it's introduction. |
|
@benjie updated |
|
I missed the CSRF stuff; we should split that out too. |
No description provided.