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

Add Metadata Decorator [WIP] #124 #190

Closed
wants to merge 2 commits into from

Conversation

DavidHooper
Copy link

Based on issues #124 and #163. Does this fit what you were thinking for the implementation?

Currently it only covers Fields + Objects until I understand you are happy with the simple implementation. From my understanding you want the Metadata decorator to be callable multiple times for the same objects/fields.

If this is what you're after, please point me in the right direction for writing some tests.

@MichalLytek
Copy link
Owner

Basically it looks ok. Metadata interface name might be confusing as we have ParamMetadata and other so it looks a bit like a base type.

In examples I would focus only on placing metadata, with fresh example, not copy-paste from the base, simple example. Copy-pasted mutations only introduce noise, not good use cases.

We should show all the possibilites of usage:

  • placing metadata decorator once and twice
  • creating own decorator for placing specific metadata key (e.g. the query-complexity case as it should be later refactored into metadata decorator usage)
  • placing in:
    • object type
    • input type
    • query
    • fields
  • reading the metadata (probably in info of resolver/middleware execution)
  • others (maybe integration with some npm package for graphql?)

And of course the usage and all possibilities have to be well described in docs.

If this is what you're after, please point me in the right direction for writing some tests.

I always try to test only the usage, so you should check if the metadata realy exist in all the types and fields after building the schema, if mutliple decorators overwrites the metadata key, etc.

You should create own test file for that, don't pollute the existing ones.

@MichalLytek MichalLytek added Enhancement 🆕 New feature or request Community 👨‍👧 Something initiated by a community labels Nov 19, 2018
@MichalLytek
Copy link
Owner

@DavidHooper
What is the progress of your work? Are you going to continue working on this feature? 😉

@DavidHooper
Copy link
Author

@19majkel94 I'm off for a while. So won't be continuing it for at least 3 weeks.

@MichalLytek MichalLytek added this to In review in Board Dec 22, 2018
@DavidHooper DavidHooper closed this Jan 9, 2019
@MichalLytek MichalLytek removed this from In review in Board Jan 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community 👨‍👧 Something initiated by a community Enhancement 🆕 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants