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

Refactors graphql-server to use dedicated plug-in directory and tests #3307

Merged
merged 97 commits into from
Sep 17, 2021
Merged

Refactors graphql-server to use dedicated plug-in directory and tests #3307

merged 97 commits into from
Sep 17, 2021

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented Aug 30, 2021 •
edited by dac09
Loading

Continuation of sunsetting of Apollo Server with the Helix and envelop graphql-server

What does this do?

  • refactoring to extract out Redwood envelop plugins into individual files
  • adds tests for each custom plugin
  • fixes error logging output, and deprecates the old way of displaying errors in the console
  • renames a few vars in globalContext for additional clarity around what this file does

Todo

  • Test logging plugin when executing a GraphQL query
  • add directive support, and add useAuth and skipAuth directives
  • implement requireAuth directive
  • Support custom directives
  • add ability to generate custom directives
  • Update template for directive test
  • Add tests for built-in requireAuth and skipAuth
  • Fix eslint plugin to allow using useXXX variable names for envelop plugin
  • Add tests for directive generator
  • Do we need to change templates for setup auth (i.e. modify the requireAuth directive) OR we changed the default requireAuth.directive that ships to have roles
  • Evaluate if there's a better lifecycle hook for validators?
  • Add schema check for directives
  • Make schema check also build time check

Requires discussion

  • [ ] Are we OK to rename the env variable SAFE_GLOBAL_CONTEXT to DISABLE_GLOBAL_CONTEXT_ISOLATION?
  • [ ] Deprecate api when writing to use createGraphQLHandler "in some way"?
  • Deprecate secure services by default
  • [ ] Do we want to import requireAuth from src/lib/auth (for backwards compatibility) - or does it make it difficult to understand where logic sits?

@dthyresson dthyresson requested a review from dac09 August 30, 2021 17:14
@dthyresson dthyresson marked this pull request as draft August 30, 2021 17:14
@dthyresson dthyresson self-assigned this Aug 30, 2021
@dthyresson
Copy link
Contributor Author

@dac09 I have integrated authDirectives in and updated sdl generators and e2e tests.

Perhaps we review, merge main, and merge and then do directive generation in a new PR?

dthyresson and others added 8 commits September 16, 2021 17:12
@dthyresson
Updates directive test snapshots
1186c8a
@dthyresson
Directive test template updates
e87a0fb
@dthyresson
Updates directive templates after review
a29a659
@dac09
Update templates and folder structures for directives
b82e3b4
@dac09
Move validateSchemaForDirectives to internal
a3e8d21 
Add internal dependency to gql-server
@dac09
Merge branch 'main' of github.com:redwoodjs/redwood into dt-dc-graphq…
6edb014 
…l-server-plugins

* 'main' of github.com:redwoodjs/redwood: (25 commits)
  Use data previously created in the scenario as input to other scenario data (#3386)
  extend scenario with support for scenario.only (#3385)
  [contributing] Add watchNodeModules option to yarn rw dev  (#3282)
  fix(testing): Fix mockCurrentUser on api side (#3369)
  Improvements to rw exec scripts (#3382)
  Refactor type generator parse method (#3381)
  Update env variable for gitpod rwfw (#3306)
  Import from specific files, not internal (#3373)
  Replace url and file loaders with asset modules (#3293)
  Support extra options in scenario to be passed to Prisma create (#3261)
  Fix: Log error during GraphQL execution (#3380)
  Fix: Generate types when generating sdl (#3378)
  Fix routes with float on Webpack dev server (#3294)
  Handle matching trailing slash (#2628)
  Surface errors on the api side (#3353)
  v0.36.4
  upgrade prisma v2.30.3 (#3355)
  v0.36.3
  fix(api-dev): Ensure dist, types and db paths are ignored on windows (#3341)
  babel-plugin-redwood-import-dir: Fix regex and Windows paths (#3311)
  ...
@dac09
Update test for global context
9ee1d57
@dac09
Fix broken tests
8e64c23
@dac09 dac09 force-pushed the dt-dc-graphql-server-plugins branch from 37f9571 to 8e64c23 Compare September 17, 2021 12:41
@dthyresson
Copy link
Contributor Author

@dac09 and @thedavidprice I added a workaround for the GraphQL schema check on windows and CI passes.

Your call on merging but I think we are ready

@thedavidprice
Copy link
Contributor

DO IT! There's no way I'm pushing that button. You two earned it. 🚀

thedavidprice
thedavidprice approved these changes Sep 17, 2021
View reviewed changes
Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

🏆

@dthyresson dthyresson merged commit b3dd5d6 into main Sep 17, 2021
@dthyresson dthyresson mentioned this pull request Sep 17, 2021
Closed
@dac09 dac09 deleted the dt-dc-graphql-server-plugins branch September 20, 2021 17:32
@thedavidprice thedavidprice modified the milestones: next-release, v0.37.0 Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dac09 dac09 dac09 left review comments

@thedavidprice thedavidprice thedavidprice approved these changes

Assignees

@dthyresson dthyresson

Labels
release:breaking This PR is a breaking change
Projects
None yet
Milestone
v0.37.0
Development

Successfully merging this pull request may close these issues.

graphql-server asynchronous beforeResolver not being handled Missing declarations for graphql-server
3 participants
@dthyresson @thedavidprice @dac09