mickhansen / graphql-sequelize

GraphQL & Relay for MySQL & Postgres via Sequelize
MIT License
1.9k stars 172 forks source link

feature request: infoToAttributes (only include attributes in the GraphQL selectionSet) #619

Closed jedwards1211 closed 4 years ago

jedwards1211 commented 6 years ago

I will be happy to make a PR. Currently resolver wastefully requests all attributes from the underlying model. I was surprised to find that this is still the case.

mickhansen commented 6 years ago

We previously had filtering of attributes but it turns out it's really counter productive since a child field might need a foreign key etc.

It's more trouble than it's worth, but you could apply the logic in before hooks.

jedwards1211 commented 6 years ago

Shouldn't it at least be an optional feature? I'd rather turn it on and make my child field resolvers throw a helpful error message about which fields need to be explicitly included for them to work.

jedwards1211 commented 6 years ago

Also what if we just avoid filtering out all of the foreign keys of a model's associations? That leaves only the case of custom field resolvers that rely on some field that's not a foreign key.

mickhansen commented 6 years ago

It could be @jedwards1211, i'd definitely consider a feature request for it as a pluggable feature (perhaps allow before to take an array of hooks, and then have a hook that filters options.attributes)

jedwards1211 commented 6 years ago

Okay, hopefully I'll find time to make a PR soon, making before take an array of hooks is a neat idea.

joewoodhouse commented 6 years ago

@mickhansen how would you go about filtering in the before hook? I can't see there's any way to access which attributes are requested in the query?

mickhansen commented 6 years ago

@joewoodhouse info holds the selection set which can be parsed

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.