graphql / graphiql

GraphiQL & the GraphQL LSP Reference Ecosystem for building browser & IDE tools.
MIT License
16.09k stars 1.72k forks source link

Editor is not showing on first render #770

Closed moti-maestro closed 4 years ago

moti-maestro commented 5 years ago

When I render , the component renders ok, but the editor is broken and you can neither type nor see what's there (although the correct elements do appear in the DOM, just with broken style). It's the same for the editor, the result section and the variables, since they all use CodeMirror.

Now, if I cause a re-render (by playing with state or moving to another page and returning), the editor loads okay and you can use the component.

Also, for some reason, I can't customize the GraphiQL component and it ignores the children nodes I supply (GraphiQL.Logo, Menu, etc.)

I reproduced the bug here: https://github.com/moti-maestro/graphiql-editor-bug

Has anyone else had this problem as well?

Thank you for your help

sevos commented 5 years ago

Try to pass empty query string parameter

 <GraphiQL fetcher={graphQLFetcher} query='' />
sevos commented 5 years ago

I take it back. Does not help

joshtldr commented 5 years ago

I ran into the same render issue where the left-hand panel would lock and I couldn't type a query in. As you mentioned above, the codemirror dependency appears to be the culprit. As a workaround I'm using a setTimeout function with a 500ms delay to lazy render the GraphiQL component. Hope this helps.

moti-maestro commented 5 years ago

@joshtldr Yea, it was also my temporary solution... I hope this issue will be fixed in the future :( Anyway, thank you!

Has anyone else ran into the non-customable components?

acao commented 5 years ago

@joshtldr @moti-maestro can you all re-create this issue with 0.13.2 by chance? we bumped codemirror and some other dependencies.

acao commented 5 years ago

@joshtldr custom logo works for me in your repository example, but the error in question persists even with the latest GraphiQL! Thanks for reporting this, I'll look into it.

acao commented 5 years ago

An interesting note, without using the setTimeout and just rendering in the default way in the docs, the bug is reproduced, but then I call g.refresh() manually from the console and it works (on componentDidMount, it seems GraphiQL exports itself to global as global.g = this). Just as an experiment I tried calling this.refresh() from componentDidMount() and this didn't help.

It also looks like Codemirror has loaded and begun manipulating the DOM on the initial, failed render. You can see its rendered all the divs with Codemirror- classes, but no

What I want to track down is why this renders properly with the bundled CDN version (what I'm used to manually testing with), but not for initial render of the library module. Answering this question will probably lead to the most performant solution, because calling CodeMirror.refresh() is obviously not ideal.

Also, to note, we plan on adding acceptance tests soon, and this is exactly the kind of issue we should be looking to detect right out the gate.

petecarapetyan commented 5 years ago

I can replicate this bug with today's version v0.13.2.

If someone could publish the specific code of a workaround - setTimeout code referenced above?? - it would be nice to use my graphiql now that I've got it running against my data model.

In case it helps for replication, I am using this very skinny codebase against an AWS AppSync Graphql instance. https://github.com/tarasowski/graphsync-browser-ide

acao commented 5 years ago

thanks @petecarapetyan! i worked for a good while this weekend on trying to find a native fix for graphiql 0.13.3. to recreate the aformentioned fix, just wrap the ReactDOM.render() in a 500ms setTimeout. it doesnt seem to be an issue in a UMD bundled version (such as the CDN bundle)

acao commented 5 years ago

This is a really tricky issue to be honest. It seems what happens is that the codemirror instance is created before all the addons and other modules are loaded in didMount for each of the editors, such as here. Possibly also before the schema is present as well. Thus why you get a bare bones instance of CodeMirror rather than the full one. This seems to happen in webpack dev server, for example, because the module loading in that runtime is much slower than in a CDN bundled script using umd/amd/etc

It appears (correct me if I'm wrong) that the only way to solve this issue properly is with a configurable module loading function prop, that allows you to leverage the dynamic imports polyfill, thus using webpack or rollup's dynamic await import() if present, and if you want to use it with IE we would just suggest using requirejs and amd/umd. Thus, it would also be up to you whether you wanted to 1) dynamically import CDN sources (which is what i see some folks do with monaco and codemirror) such as unpkg /making a cdnjs bundle for codemirror-graphql OR 2) locally bundle ones from node_modules, thus leveraging webpack's dynamic import via code splitting

Either of these configurations would in theory work with webpack or rollup.

So, each of the three editor components would have this (or we would abstract this to a utility):

async componentDidMount(){
  const CodeMirror = await dynamicImport('path/to/codemirror')
  await Promise.all(modulePaths.map(dynamicImport))
  this.editor = new CodeMirror(this._editorEl, {...configs here})
}

(this is a simplified version without config as described above)

It seems that our original browserify build setup doesn't have an issue with loading these with requirejs either, because in that build context the amd/umd (not sure which it uses yet, I'm new here) modules are already bundled, rather than being requested over the wire.

An interesting note, while researching monaco-editor, I found that they actually have a webpack plugin for generating webworkers to serve up the many modules really quickly, in a non-blocking fashion.

acao commented 5 years ago

@petecarapetyan I tried out your repo, and just removed withAuthenticator to test it quickly, and it seemed everything loaded ok? possibly the component life cycle flow is different with withAuthenticator/etc. But so far it looks fine on the initial, standalone mount? what components don't seem to be loading properly?

petecarapetyan commented 5 years ago

@acao in response to your question: withAuthenticator is an HOC that wraps the component with an AWS COGNITO login so that the graphql has the necessary credentials before firing.

The screenshot below should show that the UI comes up nicely having done the proper introspection, but that it lacks the editor component.

I am in the process of wrapping the render in a timeout now, to see if that solves the problem.

Screen Shot 2019-06-25 at 6 40 14 AM
acao commented 5 years ago

it looks like your query editor is there, the line numbering is present. is it not autocompleting the schema as you type a query? if so, it may still be the race condition, that some but not all of the codemirror addons and helpers have been instantiated. can you try g.refresh() in your browser console if the editor on the left doesnt work?

petecarapetyan commented 5 years ago

In every working demo I have seen the QUERY VARIABLES panel is below the editor panel, but I yield to you if I am mis-informed - I have yet to get my own working so I can test it.

g.refresh() does nothing, other than print 'undefined' to the console.

g.getQueryEditor() does, however, return a nice array of objects, which would seem to indicate that it exists, even if not displayed.

acao commented 5 years ago

I see, looks like you have a partially instantiated Codemirror instance then, so you are having the same issue as others above

petecarapetyan commented 5 years ago

So if g.refresh() doesn't work for me, then am I correct concluding that neither should setTimeout? I still haven't figured out the right code for render()ing on setTimeout().

petecarapetyan commented 5 years ago

@acao Here's an odd piece of information that may or may not help:

MISSING EDITOR APPEARS!

I could tell it was the missing editor right away, because those comments that show, when it's up.

Hope this gives you some extra insights.

acao commented 5 years ago

thanks @petecarapetyan . that is certainly interesting. may have to do with something involving re-renders or hot module loading

Here's what I used to get @moti-maestro 's example working above: https://github.com/moti-maestro/graphiql-editor-bug

setTimeout(() => ReactDOM.render(<GraphiQL fetcher={fetcherFunction} />), 500)
acao commented 5 years ago

@moti-maestro your original repo makes the bug easily reproducible for the dev build, but note that for the production build it works fine. with: https://github.com/moti-maestro/graphiql-editor-bug NODE_ENV=development yarn start : bug is visible yarn start: bug is not visible (note that you have to mess with the webpack config to get the production build working)

ashaninBenjamin commented 5 years ago
class CGraphiql extends React.Component {
  graphiql = React.createRef();

  graphQLFetcher = (graphQLParams) => {
    return fetch(graphqlPath, {
      ...params
      body: JSON.stringify(graphQLParams)
    }).then(response => {
      this.graphiql.current.refresh(); // <<---- refresh
      return response.json()
    });
  }

  render() {
    return <GraphiQL ref={this.graphiql} fetcher={this.graphQLFetcher} />;
  }
};

When graphql schema was loaded, refresh graphiql.

petecarapetyan commented 5 years ago

@ashaninBenjamin I can get the above code to do its refresh(), that works! Thanks.

I just can't get that refresh() to fix the problem. I still get a missing editor component. Tested in both chrome and firefox just to make sure.

gratis-gemini commented 5 years ago

@petecarapetyan , I tried everything proposed as a solution, finally what worked for me was to wrap the Graphiql component in a div block with the style: { display: 'flex', height: '100vh'}. You may give this a try.

let divStyle = { display: 'flex', height: '100vh'}; return ( \<div style = {divStyle}> \<CustomGraphiQL fetcher={this.customFetcher}/> \</div> );

swashata commented 5 years ago

Thank you @ashaninBenjamin it works.

acao commented 5 years ago

as a quick test in your browser console to see if this solution will work for you, g.refresh() should work if globals are exported. as far as the long term fix, this seems to be an issue with codemirror in general, with complex multi layered modes, so we might not have a stable resolution to this until we get to a stable release of graphiql on monaco, which will come with a lot of other perfomance improvements, webworkers, etc

barbalex commented 4 years ago

I had this working until v0.14.2. Every version since seems to have this issue.

I finally got v0.17.0 to work with @ashaninBenjamin 's trick.

Here is a version of that using functions instead of classes:

import React, { useCallback, useRef } from 'react'
import GraphiQL from 'graphiql'
import get from 'lodash/get'
import 'graphiql/graphiql.css'
import styled from 'styled-components'
import ErrorBoundary from 'react-error-boundary'

import graphQlUri from '../../modules/graphQlUri'

const Container = styled.div`
  height: calc(100vh - 64px);
`
const LoadingContainer = styled.div`
  padding: 10px;
`

const GraphIql = ({ dataGraphData }) => {
  const loading = get(dataGraphData, 'loading', false)

  const myGraphiQL = useRef(null)

  const graphQLFetcher = useCallback(
    graphQLParams =>
      fetch(graphQlUri(), {
        method: 'post',
        headers: { 'Content-Type': 'application/json' },
        body: JSON.stringify(graphQLParams),
      }).then(response => {
        // need to refresh due to issue in graphiql/it's dependencies
        // see: https://github.com/graphql/graphiql/issues/770#issuecomment-507943042
        myGraphiQL.current.refresh()
        return response.json()
      }),
    [],
  )

  if (loading) return <LoadingContainer>Lade Daten...</LoadingContainer>

  return (
    <ErrorBoundary>
      <Container>
        <GraphiQL ref={myGraphiQL} fetcher={graphQLFetcher} />
      </Container>
    </ErrorBoundary>
  )
}

export default GraphIql
acao commented 4 years ago

@barbalex i think the reason is that in 0.14.2 we switched to inline requires inside componentDidMount so that we could support SSR. this must have invoked a race condition in some contexts.

I'd like to point out two things:

A) your fetcher should not need to refresh() on every request like that. when i was able to re-create this bug, only one call to refresh() after browser mount and codemirror libs had loaded was it needed. Thus why we were setting the brief setTimeout. I'm fairly sure using refresh() that often could end up clobbering some of the editor state too, such as highlighting or the current completion context.

B) check out our webpack example, because there we get working builds and the dev server is working without having to add anything special for this. It may have to do with the version of webpack or the particular webpack config you are using if you still have this issue.

If we can't recreate this issue in our webpack example, we might have to close this ticket.

barbalex commented 4 years ago

@acao

Thanks for pointing that out.

A) I can use this like this:

useEffect(() => {
    setTimeout(() => myGraphiQL.current.refresh(), 500)
  }, [])

instead. But how can I be sure the 500ms delay will always be enough? Might there not be edge cases?

B) Our project uses an up-to-date version of gatsby.js. Nothing unusual when it comes to webpack, it would seem.

$ yarn why webpack
yarn why v1.19.0
[1/4] Why do we have the module "webpack"...?
[2/4] Initialising dependency graph...
[3/4] Finding dependency...
[4/4] Calculating file sizes...
=> Found "webpack@4.41.0"
info Reasons this module exists
   - "react-scripts" depends on it
   - Hoisted from "react-scripts#webpack"
info Disk size without dependencies: "2.3MB"
info Disk size with unique dependencies: "7.67MB"
info Disk size with transitive dependencies: "23.41MB"
info Number of shared dependencies: 128
=> Found "gatsby#webpack@4.41.2"
info This module exists because "gatsby" depends on it.
Done in 1.48s.
acao commented 4 years ago

@barbalex can i ask, with a normal production mode webpack 3 bundle (minfied, etc) are you able to see this bug? if you disable the timeout altogether?

the reason i ask is the one time i was able to reproduce this was with webpack dev server. using the config provided above, but then the bundled output worked fine. i wonder if its a race condition with codemirror modules being loaded more slowly by the webpack dev server. ive seen this cause issues with other projects that don't involve codemirror.

barbalex commented 4 years ago

@acao

This is a production build without using myGraphiQL.current.refresh() at all: https://ae2-a55347c25.now.sh/graphiql. It does not show this bug.

In dev mode it does.

So it seems the bug only occurs in dev mode 🎉 At least in my case.

mengyi-dev commented 2 years ago

image After, I installed wpgraphql it not working, it always loading. How what should I do?

thomasheyenbrock commented 2 years ago

Hey @mengyer34 👋 WPGraphQL maintains its own custom GraphQL IDE based on GraphiQL, I would suspect that the issue is not related to this codebase. (Also, I just tried it on a fresh WP site and don't see any issues with the IDE in WPGraphQL.)