-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Separate interfaces from implementations for exported classes #3616
Conversation
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
eb98ae0
to
18e979f
Compare
This PR is meant to provide a direction and proof of for removing the reliance on For example, moving BREAK into the visitor method argument certainly could be its own change. |
tests not running, see #3614 (comment) |
Note that we still warn (in development) if the objects do not conform to the GraphQL version within the peer dependency, by branding the version into the above object instances. The check is performed only at development, but the branding, for simplicity, happens in production as well. It would be good to run benchmarks. It might pay to research whether we can skip the branding in production, which might require a build step to remove the branding. |
Note that the current PR changes the class names to To lighten the breaking change set, we could keep the class names as |
0e6b059
to
bb17f06
Compare
= allows for cross-realm/worker GraphQL = allows for custom implementations of the GraphQL entity implementations, e.g. the `execute` function from GraphQL can be used for any object that implements the `GraphQLSchema` interface, even if does not conform to the default `GraphQLSchemaImpl` class. = currently provides symbols for predicates for the following interfaces: `GraphQLScalarType`, `GraphQLObjectType`, `GraphQLInterfaceType`, `GraphQLUnionType`, `GraphQLEnumType`, `GraphQLInputObjectType`, `GraphQLList`, `GraphQLNonNull`, `GraphQLDirective`, `GraphQLSchema`, and `Source`, i.e. all classes that made internal use of the `instanceOf` method. = the `instanceOf` method now uses symbols instead of `instanceof`. = the exported BREAK object is now passed into the visitor function to allow for cross-compatibility between `graphql` library instances.
Note that I removed the comment about how this can enable cross-realm graphql. That is because I was referring to constructing a schema in one thread, executing in another. It turns out that (with more research I could have discovered that) it is impossible to pass functions to a worker, so although a worker thread can receive a request and return a result, there is no way to pass GraphQL*Type objects across threads. See #2801 (comment) -- thanks @IvanGoncharov for pointing this out. This PR could be used to enable esm/cjs compatibility (avoiding/embracing the dual package hazard), but that can be saved in a more lightweight way by #3617 or some other solution. So the only remaining real reason to enable this PR is to allow drop in replacements for schema creation, validation, execution, etc. So, I am closing this PR for now, because the value of this has significantly decreased in my mind, and if I am able to get in some of the remaining refactors from |
Just a note, in TS you can "implement class":
The downside, you need to implement all private properties. |
= allows for custom implementations of the GraphQL entity implementations, e.g. the
execute
function from GraphQL can be used for any object that implements theGraphQLSchema
interface, even if does not conform to the defaultGraphQLSchemaImpl
class.= currently provides symbols for predicates for the following interfaces:
GraphQLScalarType
,GraphQLObjectType
,GraphQLInterfaceType
,GraphQLUnionType
,GraphQLEnumType
,GraphQLInputObjectType
,GraphQLList
,GraphQLNonNull
,GraphQLDirective
,GraphQLSchema
, andSource
, i.e. all classes that made internal use of theinstanceOf
method.= the
instanceOf
method now uses symbols instead ofinstanceof
.= the exported BREAK object is now passed into the visitor function to allow for cross-compatibility between
graphql
library instances.