Arguments for fields of interfaces #284

kl4n4 · 2019-03-21T20:39:33Z

Implements arguments for interfaces (#260 and #261)

allows for GraphQL schemas like the following to be defined:

interface Article {
  id: ID!
  headline: String
  image(height: Float!, width: Float!): String!
}

type WebArticle implements Article {
  id: ID!
  headline: String
  image(height: Float!, width: Float!): String!
  url: String!
}

using the following syntax:

@InterfaceType()
abstract class Article {
    @Field(type => ID)
    id: string;

    @Field({ nullable: true })
    headline?: string;

    @Field()
    image1(@Arg("width") width: number, @Arg("height") height: number): string {
        return `http://lorempixel.com/${width}/${height}/`;
    }
}

@ObjectType({ implements: Article })
class WebArticle extends Article {
    @Field()
    url: string;
}

codecov · 2019-03-21T20:42:48Z

Codecov Report

Merging #284 into master will increase coverage by 0.32%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #284      +/-   ##
=========================================
+ Coverage   94.38%   94.7%   +0.32%     
=========================================
  Files          69      69              
  Lines         979     982       +3     
  Branches       74      76       +2     
=========================================
+ Hits          924     930       +6     
+ Misses         52      49       -3     
  Partials        3       3

Impacted Files	Coverage Δ
src/schema/schema-generator.ts	`96.46% <100%> (+0.05%)`	⬆️
src/schema/utils.ts	`100% <0%> (+17.64%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb956aa...5e3bc44. Read the comment docs.

MichalLytek · 2019-03-24T17:13:17Z

@kl4n4
Please rebase your changes, I've made some tweaks in readme and force pushed them on master.

MichalLytek

We should also update changelog and the most important: update the docs to add info that now you can do such things with interface fields and arguments 😉

tests/functional/interfaces-with-arguments.ts

README.md

kl4n4 · 2019-03-26T22:48:36Z

currently there is an issue with resolving fields of interfaces when using inline arguments. I will investigate and fix this ASAP but I already pushed the tests for reference. As soon as this is fixed I will update the change log and docs.

and other missing properties

…ce type

kl4n4 · 2019-03-28T21:11:00Z

Hi @19majkel94, I now fixed all open issues, updated the changelog, added a small doc part and extended the interface example.
I hope now everything looks okay to you - if you still have any issues or suggestions, please let me know. Otherwise I hope this PR helps out one or two other users ;)

MichalLytek

Great work! 💪
There's a few issues I would like to ask you to fix 😉

docs/interfaces.md

examples/interfaces-inheritance/person/person.type.ts

tests/functional/interfaces-with-arguments.ts

MichalLytek · 2019-04-20T12:12:28Z

@kl4n4
Are you going to continue working on this PR? 😉

kl4n4 · 2019-04-20T12:27:50Z

sry for the hiatus - my work&projects kept me busy lately but I should find the time now during the easter break.

kl4n4 · 2019-04-27T09:43:35Z

@19majkel94 unfortunately it again took me a little longer than expected to finish this. But now it should be done :) I added additional tests for the use case you described in your comments, changed the example back to the way you intended it and adapt everything I could find and think of according to your comments.

please just let me know, if I missed something or there are still remaining issue with the way I implemented this feature.

docs/interfaces.md

examples/interfaces-inheritance/person/person.type.ts

MichalLytek · 2019-04-28T16:07:40Z

tests/functional/interfaces-with-arguments.ts

+          return `${intValue1}-${intValue2}`;
+        }
+        @Field()
+        implemetingObjectTypeFieldArgumentsType(@Args() args: SampleArgs1): string {


MichalLytek · 2019-04-28T16:13:04Z

tests/functional/interfaces-with-arguments.ts

+      expect(sampleField.args).toBeDefined();
+      expect(sampleField.args.length).toEqual(2);
+      expect(
+        sampleField.args.every(arg => ["intValue1", "intValue2"].includes(arg.name)),


Less magic, more explicitness - please use sampleField.args.find to get two variables and normal assertion, just like in resolvers test suite, instead of every, includes and toBeTruthy.

MichalLytek · 2019-04-28T16:14:05Z

tests/functional/interfaces-with-arguments.ts

+
+      @ArgsType()
+      class SampleArgs1 {
+        @Field(type => Int, { nullable: true, defaultValue: 50 })


Remove the nullable and defaultValue noise if it's not needed or rename the args to reflect that if it's a part of a test case.

Maybe inlineIntValue1 and argsIntValue1 to distinguish the inline from class args in tests.

MichalLytek · 2019-04-28T16:15:44Z

tests/functional/interfaces-with-arguments.ts

+    });
+
+    it("should have proper arguments for the interface field with inline arguments", async () => {
+      const sampleField = (schemaIntrospection.types.find(


Please use temporary variables instead of non-readable long chaining with types assertion. sampleField can be a variable initialized in beforeAll.

MichalLytek · 2019-04-28T16:26:39Z

tests/functional/interfaces-with-arguments.ts

+      }
+
+      @ObjectType({ implements: SampleInterface1 })
+      class SampleImplementingObject1 implements SampleInterface1 {


The ObjectType extends InterfaceType is a nice pattern to reduce the boilerplate of redeclaring fields, we should support that too as it's handy with sharing implementation of interface field resolver. So please test that behavior in both schema and functional way and also check overriding the inline field resolvers methods from base interface.

chrisdostert · 2019-06-09T16:19:46Z

is it possible to get this merged? I'm also eagerly waiting on it. Thanks @kl4n4 and @19majkel94 !

MichalLytek · 2019-06-17T17:51:07Z

is it possible to get this merged?

@chrisdostert
If you have some time you can help and change the requested parts of the code 💪

chrisdostert · 2019-10-26T17:37:31Z

@MichalLytek I hate this answer which is why I didn't respond right away, but I don't have the time to dig into this project to get comfortable enough to contribute : / . I also have FOSS projects outside of my day & night jobs that suck up all my extra time
(https://github.com/opctl and https://github.com/opspec-pkgs to name a couple) .

Going through issues, multiple have been filed asking for this functionality. Is it possible to get the PR in so people can start using the functionality, and touchups to the implementation can always be revisited?

I would hate to see the amount of work @kl4n4 has gone to go to waste : /

chrisdostert · 2020-02-12T05:14:07Z

@MichalLytek @kl4n4 any update on this?

MichalLytek · 2020-03-15T10:31:25Z

Due to lack of "Allowing changes to a pull request branch created from a fork", I am forced to close this PR in favor of the revamped #579 🔒

MichalLytek · 2020-03-23T18:39:14Z

#579 has been merged! 🎉

Thank you very much @kl4n4 for your work on this PR ❤️
It's a bit sad that you haven't find time to finish this PR and a bit shame for me that it took one year to add support for this simple feature 😞

MichalLytek and others added 3 commits March 18, 2019 21:59

docs(readme): add opencollective backers section

afe2b94

create test for intended behaviour

e2d7f95

handle Args decorator for fields of graphql interfaces

0f4077b

kl4n4 changed the title ~~Feature/arguments for interface fields~~ Arguments for fields of interfaces Mar 21, 2019

MichalLytek force-pushed the master branch 2 times, most recently from 01796f6 to be55e3c Compare March 23, 2019 18:39

MichalLytek self-requested a review March 24, 2019 17:09

MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community labels Mar 24, 2019

MichalLytek requested changes Mar 24, 2019

View reviewed changes

tests/functional/interfaces-with-arguments.ts Outdated Show resolved Hide resolved

tests/functional/interfaces-with-arguments.ts Show resolved Hide resolved

tests/functional/interfaces-with-arguments.ts Outdated Show resolved Hide resolved

Thomas Klaner added 2 commits March 24, 2019 21:28

Merge branch 'pr/1'

dacfff6

Merge branch 'master' into feature/arguments-for-interface-fields

22f2b97

MichalLytek reviewed Mar 24, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

Thomas Klaner added 3 commits March 26, 2019 22:47

fix incorrect merge

3d0a2b1

choose more meaningfull testing names for the tested fields

d394eb2

add functional tests for field argument resolving

cfa66f5

MichalLytek force-pushed the master branch from 01ee1fe to c5f6b00 Compare March 27, 2019 20:32

Thomas Klaner added 8 commits March 28, 2019 20:32

split into separate test cases

bb8a918

remove console.log from test

ce810a1

fix missing resolvers for interface fields

536647c

and other missing properties

update changelog

bb05d53

add simple documentation for fields with arguments on GraphQL interfa…

837f660

…ce type

extend interface example

1e66138

Merge remote-tracking branch 'upstream/master'

ef10058

Merge branch 'master' into feature/arguments-for-interface-fields

4451bf1

MichalLytek requested changes Mar 30, 2019

View reviewed changes

update naming of variables and types

5e5f68e

Thomas Klaner added 3 commits April 20, 2019 14:42

Merge remote-tracking branch 'upstream/master'

c3095bb

Merge branch 'master' into feature/arguments-for-interface-fields

3c0b20f

implement graphql interfaces instead of extending them

8c5bd05

MichalLytek force-pushed the master branch from 0842ff6 to cb956aa Compare April 24, 2019 17:55

Thomas Klaner added 3 commits April 27, 2019 11:34

add tests for nullable arguments and default values

5c7a39a

add tests for field resolvers

cf7e232

remove force flag for new test cases

18a66f4

Merge branch 'master' into feature/arguments-for-interface-fields

5e3bc44

MichalLytek requested changes Apr 28, 2019

View reviewed changes

MichalLytek force-pushed the master branch from 0648b22 to 9af3077 Compare September 21, 2019 15:42

MichalLytek force-pushed the master branch from c42e0e2 to 1ed5902 Compare March 6, 2020 20:40

This was linked to issues Mar 15, 2020

Specifying arguments on interfaces #261

Closed

[Feature request] Resolvers for interfaces #260

Closed

MichalLytek mentioned this pull request Mar 15, 2020

Defining arguments and implementing resolvers for interface types fields #579

Merged

This was unlinked from issues Mar 15, 2020

Specifying arguments on interfaces #261

Closed

[Feature request] Resolvers for interfaces #260

Closed

MichalLytek closed this Mar 15, 2020

MichalLytek added the Wontfix ❌ This will not be worked on label Mar 15, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Arguments for fields of interfaces #284

Arguments for fields of interfaces #284

kl4n4 commented Mar 21, 2019 •

edited

Loading

codecov bot commented Mar 21, 2019 •

edited

Loading

MichalLytek commented Mar 24, 2019 •

edited

Loading

MichalLytek left a comment

kl4n4 commented Mar 26, 2019

kl4n4 commented Mar 28, 2019

MichalLytek left a comment

MichalLytek commented Apr 20, 2019

kl4n4 commented Apr 20, 2019

kl4n4 commented Apr 27, 2019

MichalLytek Apr 28, 2019

MichalLytek Apr 28, 2019

MichalLytek Apr 28, 2019

MichalLytek Apr 28, 2019

MichalLytek Apr 28, 2019

MichalLytek Apr 28, 2019

chrisdostert commented Jun 9, 2019

MichalLytek commented Jun 17, 2019

chrisdostert commented Oct 26, 2019

chrisdostert commented Feb 12, 2020

MichalLytek commented Mar 15, 2020

MichalLytek commented Mar 23, 2020 •

edited

Loading

Arguments for fields of interfaces #284

Arguments for fields of interfaces #284

Conversation

kl4n4 commented Mar 21, 2019 • edited Loading

codecov bot commented Mar 21, 2019 • edited Loading

Codecov Report

MichalLytek commented Mar 24, 2019 • edited Loading

MichalLytek left a comment

Choose a reason for hiding this comment

kl4n4 commented Mar 26, 2019

kl4n4 commented Mar 28, 2019

MichalLytek left a comment

Choose a reason for hiding this comment

MichalLytek commented Apr 20, 2019

kl4n4 commented Apr 20, 2019

kl4n4 commented Apr 27, 2019

MichalLytek Apr 28, 2019

Choose a reason for hiding this comment

MichalLytek Apr 28, 2019

Choose a reason for hiding this comment

MichalLytek Apr 28, 2019

Choose a reason for hiding this comment

MichalLytek Apr 28, 2019

Choose a reason for hiding this comment

MichalLytek Apr 28, 2019

Choose a reason for hiding this comment

MichalLytek Apr 28, 2019

Choose a reason for hiding this comment

chrisdostert commented Jun 9, 2019

MichalLytek commented Jun 17, 2019

chrisdostert commented Oct 26, 2019

chrisdostert commented Feb 12, 2020

MichalLytek commented Mar 15, 2020

MichalLytek commented Mar 23, 2020 • edited Loading

kl4n4 commented Mar 21, 2019 •

edited

Loading

codecov bot commented Mar 21, 2019 •

edited

Loading

MichalLytek commented Mar 24, 2019 •

edited

Loading

MichalLytek commented Mar 23, 2020 •

edited

Loading