fix(graphql-transformer-common): pluralization for plural types #7030
Conversation
|
@SwaySway can you have a look please? Also the |
Codecov Report
@@ Coverage Diff @@
## master #7030 +/- ##
=======================================
Coverage 56.31% 56.31%
=======================================
Files 445 445
Lines 21806 21806
Branches 4363 4363
=======================================
Hits 12280 12280
Misses 8711 8711
Partials 815 815 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
It looks like #4224 (comment) requested a feature flag with this work.
|
|
||
| export function plurality(val: string): string { | ||
| if (!val.trim()) { | ||
| return ''; | ||
| } | ||
| if (pluralize.isPlural(val.trim())) { |
There was a problem hiding this comment.
It looks like this module can actually do the conversion to plural for you. Is there any reason not to use that functionality instead?
Side note: it might be worth putting val.trim() in a variable since it's called a few times now.
There was a problem hiding this comment.
Sure. I am pushing the changes soon.
There was a problem hiding this comment.
I think this needs to be behind a feature flag because if someone currently has a model named Tomatoes and they are depending on the list operation being called listTomatoess this will break their frontend.
We should also pin the version of pluralize because an update could potentially change the pluralization of a word which could break someones model transformation
|
Closing in favor of #7258 |
Description of changes
Added a check for pluralization of list resolvers for plural types
Issue #, if available
Fixes #4224
Description of how you validated changes
Tested using sample project GraphQL schema
Checklist
yarn testpassesBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.