WIP: feat: Add migration context to enable GraphiQL refactoring #1380
Conversation
| @@ -113,7 +113,7 @@ export const schemaReducer: SchemaReducer = ( | |||
| */ | |||
|
|
|||
| export type SchemaProviderProps = { | |||
| config: SchemaConfig; | |||
| config?: SchemaConfig; | |||
There was a problem hiding this comment.
This already provided a default so it seems it'd be fine to have as optional.
| export const GraphiQL: React.FC<GraphiQLProps> & | ||
| GraphiQLStaticProperties = props => { | ||
| return ( | ||
| <MigrationContextProvider> | ||
| <GraphiQLInternals {...props} /> | ||
| </MigrationContextProvider> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
I know this and the changes that follow are a bit ugly. I'm trying to preserve the api without requiring an extra wrapper which required some... creativity.
| interface GraphiQLStaticProperties { | ||
| formatResult: (result: any) => string; | ||
| formatError: (rawError: Error) => string; | ||
| Logo: typeof GraphiQLLogo; | ||
| Toolbar: typeof GraphiQLToolbar; | ||
| Footer: typeof GraphiQLFooter; | ||
| QueryEditor: typeof QueryEditor; | ||
| VariableEditor: typeof VariableEditor; | ||
| ResultViewer: typeof ResultViewer; | ||
| Button: typeof ToolbarButton; | ||
| ToolbarButton: typeof ToolbarButton; | ||
| Group: typeof ToolbarGroup; | ||
| Menu: typeof ToolbarMenu; | ||
| MenuItem: typeof ToolbarMenuItem; | ||
| } |
There was a problem hiding this comment.
Gotta be a better way to do this.
There was a problem hiding this comment.
we're about to drop this interface for 1.0.0 so don't worry about it too much:
#1165
|
Note that the context isn't as of yet actually being used. To access the schema provider you'd use |
| @@ -1266,6 +1296,8 @@ export class GraphiQL extends React.Component<GraphiQLProps, GraphiQLState> { | |||
| }; | |||
| } | |||
|
|
|||
| GraphiQLInternals.contextType = MigrationContext; | |||
There was a problem hiding this comment.
This is the magic that enables this.context.schema inside the class component.
There was a problem hiding this comment.
oh yes, i remember it used to work this way with the old context API too, amazing how little the class component interface changed
acao
changed the title
* Initial migration context wrapper * Pull out other static properties * Fix refs bug by properly accessing current
This PR in essence wraps the entirety of GraphiQL in an aggregated
context provider to which other context providers can be attached.
The short term goal of this is to separate out what is now GraphiQL state
into separate, smaller contexts which can be hooked into my plugins, etc
Being as we must ensure the provider is wrapped around GraphiQL I had to make
a facade component to do the wrapping and proxy props to the actual class.