search

CMeeg / gridsome-source-kentico-kontent

Kentico Kontent data source plugin for Gridsome.
MIT License
6 stars 4 forks source link

Resolver for Asset.url must have a "type" property. #12

Closed markcoole closed 4 years ago

markcoole commented 4 years ago

This is just adding a default "type" of string so that it doesn't break.

For testing purpose I have tried running "gridsome develop" with an asset included, excluded and without the asset type used in the Kentico. This is is just a work around to get the solution to build without the use of images on the site. I don't know if this has any implications elsewhere but my project is running fine now. Please do check ok and feedback if issues.

markcoole commented 4 years ago

Noticed description was used in the documentation but wasn't pulled through with a default description. This would stop the query used in the documentation from working. This fixes it but unsure if there was a reason for it being omitted. Please reject/comment if so.

johncherba commented 4 years ago

Any chance we can get this PR into the build? I am running with this fix and it works. btw thank you for this fix and npm!

CMeeg commented 4 years ago

Hi @markcoole,

First off - thank you for submitting this PR, and sorry it has taken me so long to get around to reviewing it.

I can reproduce the issue where Gridsome throws an error Resolver for Asset.url must have a "type" property., and verify that your first commit fixes that issue. This has been raised a separate issue (#11) so thank you for fixing that :)

I cannot reproduce an issue related to the second commit that adds description to the url resolver arguments. I don't believe this should be an issue as the resolver definition is separate to the Asset type definition itself. If I have understood your comment correctly about this change it's related to the asset query in the docs? If I have got that right then the description field in the query belongs to the Asset type, but the resolver args are unique to the url field - you can see a couple of them being used in that example - width and format - in brackets after the url field.

Of course, if you have had an issue that has been fixed by adding description to the resolver please let me know so I can reproduce it.

Otherwise I am happy to accept the PR, but can I ask two things, please:

Thanks again, Chris

markcoole commented 4 years ago

Hey @CMeeg,

I've removed description getAssetSchemaResolvers() method and changed the branch over to develop. Noticed this has pulled in a few release commits. If you'd rather I can create a pull request from my develop rather than my master?

CMeeg commented 4 years ago

Thanks for doing that @markcoole. No worries, I have squashed the commits so that has ironed things out nicely. I'll work on pushing a new release soon with your fix included.

CMeeg commented 4 years ago

This fix has been included in 0.4.1. Thanks again for your contribution @markcoole.

johncherba commented 4 years ago

@CMeeg and @markcoole Just tried the 0.4.1 release and works great! Thanks so much!!!