Skip to content
This repository has been archived by the owner on Mar 4, 2023. It is now read-only.

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

Merged
merged 10 commits into from
Apr 13, 2020
Merged

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

merged 10 commits into from
Apr 13, 2020

Conversation

markcoole
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Owner

CMeeg commented Apr 12, 2020

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:

  • Can you change the target branch to develop; and
  • Can you remove the description commit if it is not causing a particular issue

Thanks again,
Chris

Remove 'description' field from getAssetSchemaResolvers() as the resolver definition is separate to the Asset type definition.
@markcoole markcoole changed the base branch from master to develop April 13, 2020 07:19
@markcoole
Copy link
Contributor Author

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 CMeeg merged commit 4e322c9 into CMeeg:develop Apr 13, 2020
@CMeeg
Copy link
Owner

CMeeg commented Apr 13, 2020

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
Copy link
Owner

CMeeg commented Apr 13, 2020

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

@johncherba
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants