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.

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

Various major fixes to schema exporting #2050

Merged
merged 36 commits into from
May 7, 2024
Merged

Various major fixes to schema exporting #2050

merged 36 commits into from
May 7, 2024

Conversation

benjie
Copy link
Member

@benjie benjie commented May 7, 2024

Description

This PR sets up testing of the exported schemas by running the integration tests again, this time by generating the schema (as before) then exporting the schema, importing it, and using the imported version for running the tests. It's really inefficient right now, making the tests really slow, but worth it for peace of mind. In future this will need to be redeveloped to find all the schema configurations and then write out those schemas in advance and use them to run the tests against.

Then, of course, there were loads of hard to figure out errors/discrepancies which I had to track down. Roughly:

  • grafast's makeGrafastSchema has been rebuilt to follow better patterns (fixing issues with enums in the process)
  • grafast's makeGrafastSchema now supports __inputPlan(){} on an input object's plans
  • graphile-export now exports the input object input plan in typeDefs mode
  • graphile-export now detects functions with additional properties (e.g. isSyncAndSafe: true) and makes sure these properties are represented in the export
  • graphile-export now outputs the serialize, parseValue and parseLiteral methods on custom scalars in typeDefs mode
  • Marks a number of other things as EXPORTABLE to fix exports
  • Removes EXPORTABLE from a couple of things to fix exports
  • Fixes an issue with our linting of the exported schema where it might be ignored sometimes
  • ConstantStep now supports .get() and .at() patterns
  • Fix enableDeferStream detection in schema export by looking at directives in schema being exported

Fixes #2043
Fixes #2049

Performance impact

Import/export only.

Security impact

Improvement: more accurate exports are safer.

Checklist

  • My code matches the project's code style and yarn lint:fix passes.
  • I've added tests for the new feature, and yarn test passes.
  • I have detailed the new feature in the relevant documentation.
  • I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
  • If this is a breaking change I've explained why.

Copy link

changeset-bot bot commented May 7, 2024

🦋 Changeset detected

Latest commit: 2a00087

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 15 packages
Name Type
grafast Patch
graphile-export Patch
graphile-build-pg Patch
graphile-build Patch
postgraphile Patch
@localrepo/grafast-bench Patch
@dataplan/json Patch
@dataplan/pg Patch
@grafserv/persisted Patch
grafserv Patch
ruru-components Patch
@localrepo/grafast-website Patch
graphile-utils Patch
pgl Patch
graphile Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@benjie benjie marked this pull request as ready for review May 7, 2024 13:07
benjie added 27 commits May 7, 2024 14:07
… additional properties set, and makes sure these properties are exported too. (Typically this is `fn.isSyncAndSafe=true` for Grafast optimization.)
@@ -12373,6 +12389,15 @@ const makeArgs64 = (args, path = []) => {
return selectArgs;
};
const resource_compound_type_computed_fieldPgResource = registry.pgResources["compound_type_computed_field"];
function UUIDSerialize(value) {
return "" + value;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

As an optimization we should have this export as toString (the source name) rather than being called UUIDSerialize everywhere. But no big deal.

Comment on lines 33442 to +33445
NestedCompoundTypeInput: {
"__inputPlan": function NestedCompoundTypeInput_inputPlan() {
return object(Object.create(null));
},
Copy link
Member Author

Choose a reason for hiding this comment

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

Would be nice to optimize this down to a method syntax.

@benjie benjie merged commit 5cb5523 into main May 7, 2024
36 checks passed
@benjie benjie deleted the schema-exports-test branch May 7, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
1 participant