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.

Sign up for GitHub

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

Jump to bottom

handle the race condition in updating operationName #344

Merged
merged 1 commit into from
Feb 27, 2017
Merged

handle the race condition in updating operationName #344

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 30 additions & 12 deletions src/components/GraphiQL.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,18 @@ export class GraphiQL extends React.Component {
if (nextSchema !== this.state.schema ||
nextQuery !== this.state.query ||
nextOperationName !== this.state.operationName) {
this._updateQueryFacts(nextQuery);
const updatedQueryAttributes = this._updateQueryFacts(
nextQuery,
nextOperationName,
this.state.operations,
nextSchema,
);

if (updatedQueryAttributes !== undefined) {
nextOperationName = updatedQueryAttributes.operationName;

this.setState(updatedQueryAttributes);
}
}

// If schema is not supplied via props and the fetcher changed, then
Expand Down Expand Up @@ -650,34 +661,41 @@ export class GraphiQL extends React.Component {

handleEditQuery = debounce(100, value => {
if (this.state.schema) {
this._updateQueryFacts(value);
this.setState(
this._updateQueryFacts(
value,
this.state.operationName,
this.state.operations,
this.state.schema,
),
);
}
this.setState({ query: value });
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combine this with the above to ensure one call to setState?

const queryFacts = this.updateQueryFacts(...);
this.setState({
  query: value,
  ...queryFacts
})

if (this.props.onEditQuery) {
return this.props.onEditQuery(value);
}
})

_updateQueryFacts = query => {
const queryFacts = getQueryFacts(this.state.schema, query);
_updateQueryFacts = (query, operationName, operations, schema) => {
const queryFacts = getQueryFacts(schema, query);
if (queryFacts) {
// Update operation name should any query names change.
const operationName = getSelectedOperationName(
this.state.operations,
this.state.operationName,
const updatedOperationName = getSelectedOperationName(
operations,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be queryFacts.operations - the operations passed in from the outside (via this.state.operations) will be derived from old information. getQueryFacts is responsible for determining that value from the query

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I'm wrong on this one. This is the previous operations, just was confused by the names

operationName,
queryFacts.operations
);

// Report changing of operationName if it changed.
const onEditOperationName = this.props.onEditOperationName;
if (onEditOperationName && operationName !== this.state.operationName) {
onEditOperationName(operationName);
if (onEditOperationName && operationName !== updatedOperationName) {
onEditOperationName(updatedOperationName);
}

this.setState({
operationName,
return {
operationName: updatedOperationName,
...queryFacts
});
};
}
}

Expand Down