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

ReferenceError: navigator is not defined (remix react) #2368

Closed
Hansanghyeon opened this issue Apr 20, 2022 · 14 comments · Fixed by #2545
Closed

ReferenceError: navigator is not defined (remix react) #2368

Hansanghyeon opened this issue Apr 20, 2022 · 14 comments · Fixed by #2545
Assignees

Comments

@Hansanghyeon
Copy link

Hi There,
I am trying to use GraphiQL with Hansanghyeon/error-remix-graphiql but I am getting following error:

There was a similar issue. #118

ReferenceError: navigator is not defined
    at /Users/imweb/@lab/blog-tutorial/node_modules/codemirror/lib/codemirror.js:18:19
    at /Users/imweb/@lab/blog-tutorial/node_modules/codemirror/lib/codemirror.js:11:83
    at Object.<anonymous> (/Users/imweb/@lab/blog-tutorial/node_modules/codemirror/lib/codemirror.js:14:2)
    at Module._compile (node:internal/modules/cjs/loader:1099:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:999:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at /Users/imweb/@lab/blog-tutorial/node_modules/codemirror/addon/hint/show-hint.js:8:9
    at Object.<anonymous> (/Users/imweb/@lab/blog-tutorial/node_modules/codemirror/addon/hint/show-hint.js:13:3)
    at Module._compile (node:internal/modules/cjs/loader:1099:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:975:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:999:19)
@thomasheyenbrock thomasheyenbrock self-assigned this May 16, 2022
@MH4GF
Copy link

MH4GF commented Jul 6, 2022

I encountered a similar error when using the GraphiQL component on next.js.
It seems like the problem is probably with SSR, and I solved it by making it load only in client.
For next.js provides functionality for dynamic import: https://nextjs.org/docs/advanced-features/dynamic-import

@thomasheyenbrock
Copy link
Collaborator

Hey @Hansanghyeon 👋 rendering GraphiQL server-side is currently to supported as @MH4GF correctly pointed out. To work around this in any React app you can also do something like this (basically exploiting the fact that useEffect does not runs during SSR):

function ClientOnlyGraphiQL() {
  const [isClientRendering, setIsClientRendering] = useState(false);
  useEffect(() => {
    setIsClientRendering(true);
  }, []);
  return isClientRendering ? <GraphiQL /> : null;
}

@AndKiel
Copy link

AndKiel commented Jul 6, 2022

I have exactly the same issue in my remix app. I cannot upgrade above "graphiql": "1.7.2" and "graphiql-explorer": "0.9.0" because of codemirror and navigator. The solution above works up to those versions and then afterwards it doesn't.

@acao
Copy link
Member

acao commented Jul 6, 2022

This has always been the case with GraphiQL unfortunately, as codemirror 5 is not designed for SSR and we have no control over that.

For some time we had codemirror 5 working with next.js naturally (why we implemented inline require/dynamic import for codemirror imports) but several years ago it stopped working in nextjs out of the box, and folks had to use the didMount hack to avoid the node rendering step

@acao
Copy link
Member

acao commented Jul 6, 2022

Yes we shouldn’t use any browser APIs except after did mount or inside useEffect(), though I guess node SSR servers go that far in the component lifecycle now?

@AndKiel
Copy link

AndKiel commented Jul 6, 2022

Because this version is the first that stopped working - [email protected]- I think for some reason #2249 is to blame.

@acao
Copy link
Member

acao commented Jul 6, 2022

But to be clear, since the PR i introduced in 2018, GraphiQL has tried it‘s best to support SSR, and continue to do so however. In fact I think we supported SSR back when we still had the pre-webpack, gulp era toolchain! The timeout workaround should at least always work as a baseline.

@thomasheyenbrock
Copy link
Collaborator

@AndKiel I just tried it out and can confirm that remix doesn't even start the dev server when you try to import something from graphiql, so the workaround is useless. You also have the right hunch, the problem is that we import some common codemirror modules top-level, see here:

export const commonCodeMirrorAddons = [
import('codemirror/addon/hint/show-hint'),
import('codemirror/addon/edit/matchbrackets'),
import('codemirror/addon/edit/closebrackets'),
import('codemirror/addon/fold/brace-fold'),
import('codemirror/addon/fold/foldgutter'),
import('codemirror/addon/lint/lint'),
import('codemirror/addon/search/searchcursor'),
import('codemirror/addon/search/jump-to-line'),
import('codemirror/addon/dialog/dialog'),
// @ts-expect-error
import('codemirror/keymap/sublime'),
];

All this stuff is dynamically imported when you import anything from graphiql or @graphiql/react. We should definitely fix that!

@acao
Copy link
Member

acao commented Jul 6, 2022

@AndKiel ahhh!! Well i made that change so that esbuild folks could build the project without inline require. SIGH. Probably next.js trying to automatically prefetch dynamic imports or something. Well maybe I’ll try out the latest next.js here soon and see what I can come up with.

But either way, it will involve only rendering the component client side, unless someone here has a magical way to render codemirror 5 on the server!

@acao
Copy link
Member

acao commented Jul 6, 2022

@thomasheyenbrock wait, where are we importing codemirror modules top level? The file you linked to shows dynamic imports, which means they are only imported when the promise is resolved

@thomasheyenbrock
Copy link
Collaborator

Not 100% sure what happens here (I'm not a bundler expert 😅 ), but what I did in #2545 fixed the Remix dev server that I used to replicate the issue locally.

@thomasheyenbrock
Copy link
Collaborator

To clear up wording ambiguity, instead of "top-level imports" I should have said "top-level dynamic imports". These promises to get fired when importing the module, by putting them inside the importCodeMirror function (which is only ever called in useEffect) we can be sure that they only fire when this code runs in the browser.

@acao
Copy link
Member

acao commented Jul 6, 2022

I understand now, it’s because the imports weren’t in a closure!

E.g. why we can do React.lazy(() => import()) but not this

so another approach, we can still specify this config at the top level if we want, but it would be a function that is called that returns the array

const addons = () => [import('something')]


function MyComponent() {
  const addonPromisesToResolve = addons()
}

sorry for the shitty iPhone psuedocode lol

i am personally a fan of keeping configuration highly visible and towards the top of the file, but I’m old school so please disregard

@thomasheyenbrock
Copy link
Collaborator

The fix was just published with @graphiql/[email protected] and [email protected]

nick-lanners added a commit to Zonos/amino that referenced this issue Jun 8, 2023
implement a workaround to prevent the cursor from jumping to the
beginning of the query window when typing quickly
this fix comes from:
graphql/graphiql#2368 (comment)
possibly doesn't work all the time, GitHub issue is not being worked on
as GraphiQL is looking to reimplement the Explorer plugin soon
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

Successfully merging a pull request may close this issue.

5 participants