Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

deprecate transforms' visitSchema in favor of SchemaVisitor's visitSchema #20

Closed
yaacovCR opened this issue Sep 1, 2019 · 4 comments
Closed

Comments

@yaacovCR
Copy link
Owner

yaacovCR commented Sep 1, 2019

HEAVILY EDITED

See #19 and #14.

visitSchema as defined in the transforms code section seems to be deprecated in favor of SchemaVisitor. SchemaVisitor even has its own visitSchema method!

The differences are as follows:

  1. Modification of original schema. SchemaVisitor version modifies the original schema, while the transforms version creates a new schema. You can now easily create a new schema prior to passing to visitSchema by using the toConfig() method, which could be done prior to visiting, or visiting could be changed to do this internally, perhaps by a variable.
  2. Method of passing hooks. SchemaVisitor allows customization by passing a derived class that overrides particular methods. The transforms version allows passing an object similar to the graphql visit() function.
  3. Types of hooks. SchemaVisitor has more customization hooks than the transforms method, i.e. you can hook into fields, arguments, and enum values. This could be added to the transforms visitSchema method with new VisitSchemaKind values like VisitSchemaKind.FIELD and VisitSchemaKind.ARGUMENT, etc.

This begs for unification.

One path forward is just to deprecate the transforms version. This would be pretty straightforward, as the transforms version is used just for to (A) initially wrap the schema with a round of delegation and (B) to modify the outer schema within a few transforms.

(A) transformSchema uses the transforms visitSchema with the stripResolvers set as true, so it wraps the original schema with new root fields that delegate to the original schema and changes the rest of the resolvers to defaultMergedResolver. This could easily be refactored to recreate the schema with toConfig and then just use addResolversToSchemas to replace resolvers.

(B) The individual transforms uses the transforms visitSchema to modify the outer schema. These transforms currently don't modify the original schema, but there is no reason why they cannot, and so it would be ok to switch to SchemaVisitor or even just direct modification.

@yaacovCR
Copy link
Owner Author

yaacovCR commented Sep 4, 2019

Turns out toConfig() just changes format, does not clone. :(

@yaacovCR
Copy link
Owner Author

yaacovCR commented Sep 9, 2019

See 3d10a57 and a5deacc.

Figured out a cloneSchema implementation in conjunction with healTypeMap extracted from healSchema. (A) taken care of, not to work on (B)...

@yaacovCR
Copy link
Owner Author

yaacovCR commented Sep 9, 2019

The ongoing refactoring will probably also include deprecation of recreateType in favor of shallow cloneType followed by healTypeMap.

recreateType is used in individual transforms, the transforms' visitSchema, and mergeSchemas, which can all be simplified greatly.

@yaacovCR
Copy link
Owner Author

Live in v6.4. have to work on docs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant