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

@rest Directive doesn't work on SSR environment #182

Closed
Richard87 opened this issue Jan 7, 2019 · 9 comments
Closed

@rest Directive doesn't work on SSR environment #182

Richard87 opened this issue Jan 7, 2019 · 9 comments

Comments

@Richard87
Copy link

Hi!

I have a @rest graphql query that works perfectly in development/browser, but when I run the same code on the server (SSR), it fails with a long cryptic error message.

(if I disable the @rest directive, everything else works flawlessly, both in browser and in SSR).

The stack trace:

(node:26034) UnhandledPromiseRejectionWarning: Error: Network error: current.forEach is not a function
    at new ApolloError (.../demo-component/node_modules/src/errors/ApolloError.ts:56:5)
    at .../demo-component/node_modules/src/core/QueryManager.ts:512:31
    at .../demo-component/node_modules/src/core/QueryManager.ts:1046:11
    at Array.forEach (<anonymous>)
    at .../demo-component/node_modules/src/core/QueryManager.ts:1045:10
    at Map.forEach (<anonymous>)
    at QueryManager.broadcastQueries (.../demo-component/node_modules/src/core/QueryManager.ts:1039:18)
    at .../demo-component/node_modules/src/core/QueryManager.ts:411:18
(node:26034) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 5)
(node:26034) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Please advice :/

@Richard87
Copy link
Author

A little more digging, I'm just trying to execute the query in NodeJS, without worrieng about SSR etc,

and have this code:

            client
                .query({query: Site, variables: {domain: "demo.example.com"}})
                .then(
                    () => console.log("Fullfilled"),
                    ({graphQLErrors, networkError, message, extraInfo}) =>
                        console.error("Rejected", graphQLErrors, networkError, message,extraInfo)
                )

With this result:

Rejected [] TypeError: current.forEach is not a function
    at /home/richard/Projects/skil/demo-component/node_modules/src/restLink.ts:671:13
    at Array.reduce (<anonymous>)
    at concatHeadersMergePolicy (/home/richard/Projects/skil/demo-component/node_modules/src/restLink.ts:664:23)
    at RestLink.request (/home/richard/Projects/skil/demo-component/node_modules/src/restLink.ts:1235:21)
    at ApolloLink.request (/home/richard/Projects/skil/demo-component/node_modules/apollo-link/src/link.ts:76:19)
    at /home/richard/Projects/skil/demo-component/node_modules/apollo-link/src/link.ts:78:26
    at ApolloLink.request (/home/richard/Projects/skil/demo-component/node_modules/src/ApolloClient.ts:139:16)
    at ApolloLink.request (/home/richard/Projects/skil/demo-component/node_modules/apollo-link/src/link.ts:76:19)
    at /home/richard/Projects/skil/demo-component/node_modules/apollo-link/src/link.ts:78:26
    at DedupLink.request (/home/richard/Projects/skil/demo-component/node_modules/apollo-link-dedup/src/dedupLink.ts:39:30) Network error: current.forEach is not a function undefined

restLink.ts:671 is this block of code:

export const concatHeadersMergePolicy: RestLink.HeadersMergePolicy = (
  ...headerGroups: Headers[]
): Headers => {
  return headerGroups.reduce((accumulator, current) => {
    if (!current) {
      return accumulator;
    }
    if (!current.forEach) {
      current = normalizeHeaders(current);
    }
    current.forEach((value, key) => {    // <-- This line here
      accumulator.append(key, value);
    });

    return accumulator;
  }, new Headers());
};

https://github.com/apollographql/apollo-link-rest/blob/master/src/restLink.ts#L671

@Richard87
Copy link
Author

Richard87 commented Jan 7, 2019

Also, if I replace the forEach that fails with this:

    Object.keys(current).forEach(key => {
      accumulator.append(key, current[key])
    })

Everything works again...

The challenge, I dont understand the purpose of the lines above:

    if (!current.forEach) {
      current = normalizeHeaders(current);
    }

Since this would always fail, and always execute normalizeHeaders()?

@tombarton
Copy link

I'm going to go out on a limb here and guess that Node is getting upset about the use of new Headers(). A new Headers instance has a forEach method on it's prototype. If the object supplied to the reduce is missing a forEach method, we execute normalizeHeaders. This determines if the variable is an instance of Headers. If not, it will generate a new set of headers, again, using new Headers(), thus causing the issue you're seeing here.

Your Object.keys(current).forEach() example is working because it is successfully converting a plain object to an array, allowing you to call .forEach() on it.

Next step debugging wise...what is the result of normalizeHeaders() in your SSR application? Take a look at what it outputs and I'm guessing you'll find that it's a plain object, rather than an instance of Headers.

As far as I'm aware, I don't believe SSR has been tested fully (see #155) but @fbartho can probably shed more light on this.

@fbartho
Copy link
Collaborator

fbartho commented Jan 10, 2019

SSR is not a directly tested or supported feature at this time. Rationale: if you have SSR, you can deploy your own Apollo GraphQL server, and you don’t need link-rest

Some people have figured it out however. And I’m not objecting to PRs that improve the state of the world there. It seems to me that we maybe just need an SSR tutorial.

@fbartho
Copy link
Collaborator

fbartho commented Jan 10, 2019

Additionally, it’s relatively common to need to polyfill Headers depending on your runtime environment. Have you tried that yet? @Richard87

@tombarton’s response is correct: the purpose of that code is to convert header hashes into the appropriate Headers class.

@Richard87
Copy link
Author

Richard87 commented Jan 10, 2019

Hi all!

Thanks for the attention to my problem ;)

The bottom part of my index.js is this:

if (global.Headers == null) {
    global.Headers = require("fetch-headers");
}

// Set up babel to do its thing... env for the latest toys, react-app for CRA
// Notice three plugins: the first two allow us to use import rather than require, the third is for code splitting
// Polyfill is required for Babel 7, polyfill includes a custom regenerator runtime and core-js
require('@babel/polyfill');
require('@babel/register')({
    ignore: [/\/(build|node_modules)\//],
    presets: ['@babel/preset-env', '@babel/preset-react'],
    plugins: [
        '@babel/plugin-syntax-dynamic-import',
        '@babel/plugin-proposal-class-properties',
        '@babel/plugin-transform-instanceof',
    ]
});

// Now that the nonsense is over... load up the server entry point
require('./server');

So I'm pretty sure I have the Header polyfill (also without the global.Headers I got a bunch of other issues)...

I already have a GraphQL server running tighly integrated in my PHP Application (API-Platform + https://webonyx.github.io/graphql-php/) So I don't want to install an extra Apollo GraphQL Server, I just want to load the React app faster :D

I have a hard time debugging restLink.ts, but I have put in a few strategic console.log but it current seems to be a plan object, but console.logmight miss some information...

Edit...

export const concatHeadersMergePolicy: RestLink.HeadersMergePolicy = (
  ...headerGroups: Headers[]
): Headers => {
  return headerGroups.reduce((accumulator, current) => {
    if (!current) {
      return accumulator;
    }

    console.log(current.constructor.name)
    if (!current.forEach) {
      current = normalizeHeaders(current);
    }
    console.log(current.constructor.name)

    Object.keys(current).forEach(key => {
      accumulator.append(key, current[key])
    })

    return accumulator;
  }, new Headers());
};

Prints out "Header" twice...

console.log(current.forEach) prints out Undefined no matter what, it seems my Headers polyfill doesn't have a forEach property?

@Richard87
Copy link
Author

My PHPStorm tells me headers have a forEach method, but when running this code in node I get a headers.forEach is not a function:

const Headers = require("fetch-headers");
const headers = new Headers();

console.log(headers)
console.log(headers.forEach)
console.log(!headers.forEach)

headers.forEach(h => console.log(h))

Output:

node /home/richard/Projects/demo-component/src/test.js 
Headers {}
undefined
true
/home/richard/Projects/demo-component/src/test.js:10
headers.forEach(h => console.log(h))
        ^

TypeError: headers.forEach is not a function
    at Object.<anonymous> (/home/richard/Projects/demo-component/src/test.js:10:9)
    at Module._compile (internal/modules/cjs/loader.js:688:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:699:10)
    at Module.load (internal/modules/cjs/loader.js:598:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:537:12)
    at Function.Module._load (internal/modules/cjs/loader.js:529:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:741:12)
    at startup (internal/bootstrap/node.js:285:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:739:3)

@Richard87
Copy link
Author

Adding this in my bootstrap file solved all my problems :D

if (global.Headers == null) {
    const fetch = require('node-fetch');
    global.Headers = fetch.Headers;
}

notice using node-fetch instead of fetch-headers solved the issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants