[Graphiql Explorer Plugin] (Next.js) Cursor/Caret jumps when editing query manually

crjm commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Current Behavior

When editing the query by hand when using the GraphiQL component together with the explorerPlugin passed in the props array, the cursor resets, and appears in the first line of the editor. I'm currently implementing the plugin as stated in the README

Expected Behavior

The caret shouldn't jump to the beginning.

Steps To Reproduce

1. Clone https://github.com/crjm/graphiql-explorer-test
2. cd into cloned repo, `npm install`
3. `npm run dev`
4. Open Explorer plugin, trigger fields
5. Start typing directly into the editor
6. The caret will jump to the first line of the editor.

Module pattern

[ ] graphiql-umd
[X] graphiql-esm
[ ] graphiql-commonjs

Environment

- GraphiQL Version: latest
- OS: Ubuntu 22.04
- Browser: Firefox
- Bundler: Next.js (Webpack 5)
- `react` Version: 18.2
- `graphql` Version: 15.8

Anything else?

No response

jonathanawesome commented 2 years ago

I can't replicate this in your repository or with StackBlitz. Are you able to take a screen capture to share?

marcosnils commented 2 years ago

I can't replicate this in your repository or with StackBlitz. Are you able to take a screen capture to share?

:wave: seems to happen in StackBlitz as well. Recording below using Mozilla Firefox 106.0.4.

This doesn't happen when using plain react. Maybe it's realted to Next SSR nature and the fact that dynamic is being used here to force client-side rendering? https://github.com/crjm/graphiql-explorer-test/blob/main/pages/index.js#L6

https://user-images.githubusercontent.com/1578458/199839723-1f14bebb-ea72-4ad3-8027-86d95a1d6747.mp4

marcosnils commented 2 years ago

@jonathanawesome just checking if you were able to reproduce :pray:

jonathanawesome commented 2 years ago

Yep, thanks...I see it now and you're probably right that it's related to the dynamic import.

I can get it to jump without interacting with the Explorer plugin, so that's not the issue.

Curious that we haven't had this reported before, I wonder if it has something to do with next 13.

Have you tried downgrading to next 12?

Have you tried with babel rather than with swc?

marcosnils commented 2 years ago

Thx for the quick reply Jonathan. It also happens in next 12. Regarding babel, we haven't tried that yet.

We'll keep doing some more tests

jonathanawesome commented 2 years ago

Here's a related conversation with a workaround.

marcosnils commented 2 years ago

Here's a related conversation with a workaround.

In our case we're using NextJS dynamic imports with without SSR capabilities as described here: https://nextjs.org/docs/advanced-features/dynamic-import#with-no-ssr. If my understanding is correct, it should be the same as the outcomes you've shared. This issue happens only when adding the explorer plugin, so I assume there must be something happening around how both libs are being bundled and initialized in the client-side when using Next.

bkuzma commented 2 years ago

I'm experiencing the cursor jump as well and I'm using neither NextJS nor SSR 😕 I am using the explorer plugin, though I saw the problem occur when I disabled it. One thing I think I noticed: when I removed the explorer plugin along with the code that keeps the user-typed query in state (this is needed for the explorer plugin, see https://github.com/OneGraph/graphiql-explorer-example/blob/master/src/App.js#L88) and sets that query as a prop on the GraphiQL instance, I couldn't reproduce the problem.

GraphiQL Version: 2.0.9
OS: Mac OS Ventura
Browser: Chrome
Bundler: Babel
react Version: 18.2.0
graphql Version: 16.6.0

davegariepy commented 1 year ago

seems like useState is not keeping up turning off onEditQuery seems to stop the cursor jump unfortunately this causes the explorer plugin checkboxes to not match subsequent manual edits in the editor, a somewhat less annoying bug

<GraphiQL
      fetcher={fetcher}
      query={query}
      // onEditQuery={setQuery}
      plugins={[explorerPlugin]}
    />

rrva commented 1 year ago

We applied the following workaround, but it does not prevent the the cursor jumps completely, it only makes them happen less often. Is this bug being worked on? It is pretty annoying.

diff --git a/src/App.tsx b/src/App.tsx
index a0658c1..68553a7 100644
--- a/src/App.tsx
+++ b/src/App.tsx
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, {useEffect} from 'react';
 import GraphiQL from 'graphiql';
 import 'graphiql/graphiql.css';
 import '@graphiql/plugin-explorer/dist/style.css';
@@ -105,8 +105,14 @@ const App = () => {
         return api != null ? api.url.substring(0, api.url.lastIndexOf("/")) : "";
     }

+    const [isClientRendering, setIsClientRendering] = useState(false);
+    useEffect(() => {
+        setIsClientRendering(true);
+    }, []);
+
     return (
         <>
+            {isClientRendering ?
             <GraphiQL fetcher={fetcher} query={query} onEditQuery={updateQuery} plugins={[explorerPlugin]} toolbar={{
                 additionalContent: (
                     <>
@@ -165,6 +171,7 @@ const App = () => {
                 ),
             }}>
             </GraphiQL>
+                : null}
             {showComparer && (
                 <Comparer
                     query={query}

jonathanawesome commented 1 year ago

We applied the following workaround, but it does not prevent the the cursor jumps completely, it only makes them happen less often. Is this bug being worked on? It is pretty annoying.

I'm not currently working on this (and my assumption is that no one else is, either), nor do I plan to. The OneGraph Explorer plugin that's in this repository is a legacy placeholder that is meant to hold folks over until we've had the time to design and develop the new "core" plugin with similar functionality (see here).

That said, this is open source software. If any one out there that is deeply impacted by this issue would like to take the time to come up with a working solution, we'd be happy to review and merge.

rrva commented 1 year ago

We applied the following workaround, but it does not prevent the the cursor jumps completely, it only makes them happen less often. Is this bug being worked on? It is pretty annoying.

I'm not currently working on this (and my assumption is that no one else is, either), nor do I plan to. The OneGraph Explorer plugin that's in this repository is a legacy placeholder that is meant to hold folks over until we've had the time to design and develop the new "core" plugin with similar functionality (see here).

That said, this is open source software. If any one out there that is deeply impacted by this issue would like to take the time to come up with a working solution, we'd be happy to review and merge.

Thanks, good to know. We will consider if we stop using explorer for a while, or at least offer a version without it.

rrva commented 1 year ago

@jonathanawesome I know little react, do you think the bug is that useState triggers unwanted re-renders? Which of these do you think is worth trying?

Rewrite the solution using:

useEditorContext
useRef
memo

acao commented 1 year ago

Does the issue seem to be fixed here?

I've decided to start deploying our graphiql + webpack w/ plugins example again so we can make these issues more easily reproducible

https://deploy-preview-3328--graphiql-test.netlify.app/webpack/

marcosnils commented 1 year ago

https://deploy-preview-3328--graphiql-test.netlify.app/webpack/

just tested in this URL and seems to be fixed here indeed.

acao commented 1 year ago

awesome to hear @marcosnils , thanks for checking it out for me. I will merge as #3330 as 0.3.0 releases of the plugins. it will cause breaking changes to the plugin implementations that will simplify plugin usage

acao commented 1 year ago

@marcosnils here is a version to test in your implementation, can you confirm it works in your implementation?

canary fix version:

@graphiql/plugin-code-exporter@0.3.0-canary-f3cc0782.0
CDN version: https://unpkg.com/browse/@graphiql/plugin-code-exporter@0.3.0-canary-f3cc0782.0/dist/index.umd.js

please follow these instructions to update your implementation with the new plugin signature.

@rrva @bkuzma @crjm try this fix version if/when you can!

acao commented 1 year ago

this fix has been released as 0.3.0!

please note the changelog again, one slight naming convention tweak to code exporter plugin while we were making a breaking change.

if you are using the module from npm, the changelog instructions will help you, and the READMEs have been updated.

if you are using the CDN bundle path, follow the updated example for explorer plugin and/or for code exporter plugin

rrva commented 1 year ago

Awesome @acao

Have not tested yet, but this is a big one for us. It prevented explorer usage so we were stuck with using both v1 and v2 in parallel. v1 had explorer, v2 can send auth headers. Looking forward to use v2 for all use cases

acao commented 1 year ago

@rrva oh dear, yes we added the headers tab in graphiql@1.2 or so I believe, but you should be able to use the latest 2.x and the @graphiql/plugin-explorer@0.3.0 together without this bug now! let us know if there are still issues in your implementation and we can re-open, but this seemed to resolve the issue for several people using either the module or UMD builds

maarcingebala commented 1 year ago

We're currently using the 3.0.6 version of GraphiQL and are still experiencing the issue. It seems to happen less often, though.

graphql / graphiql