Allow objects with __toString in IDType

[enumag]

I'm using Uuid objects from ramsey/uuid as IDs and this library complains because it's an object.

If you think I should do it differently (like using my own GraphQL type for uuids instead of ID) please enlighten me.

[vladar]

Thanks for the PR. This is basically what we discussed in #203 (comment)

I would appreciate if you add couple test cases in https://github.com/webonyx/graphql-php/blob/master/tests/Type/ScalarSerializationTest.php

Then it will be ready for merging.

[coveralls]

Coverage remained the same at 92.838% when pulling 25e341e on enumag:patch-1 into 3536280 on webonyx:master.

[enumag]

I can do that. Just not sure where to put that class with __toString which is needed for this test.

Also do you think this is a good approach? I'm new to GraphQL so I'm unsure if it's expected for every GraphQL object to have an ID type field or if it's ok or even preferred to use a custom type for things like UUID (custom type might help with validation for example). I'm not even sure if it's good to use UUIDs with GraphQL in the first place.

[vladar]

Drop it in the same folder for now, just name with a Stub suffix. I'll move it later when working on next release. Note that you may have to require it manually in test class - I just don't remember if we have autoloading for test classes.

As for using objects for ids - it should be OK. One problem that this solution doesn't solve though is parsing. Some ID types could be transformed back to object from string (like Relay's global id where you often concat type name and type instance id).

But this is out of the scope of this PR. For now we should at least support __toString casting, so your PR is ok.

[enumag]

Ok, I'll add the test. Still do you think I should write an UuidType for GraphQL? I can share it of course if I write it.

[vladar]

If you use it for entity identification then ID is what you need. But if you rely on the fact that it is UUID anywhere in your code, then you should have a separate type.

As a rule of thumb - if you can safely replace your ids with any other unique string without breaking clients - then it qualifies as ID type. Otherwise use custom scalars.

Also in a future version of GraphQL we can see this feature - graphql/graphql-js#914 which can make custom scalars more convenient.

[coveralls]

Coverage increased (+0.03%) to 92.866% when pulling 9e2c1da on enumag:patch-1 into 3536280 on webonyx:master.

[enumag]

Test added.

Your answer is a bit unhelpful... I use UUIDs as entity identification obviously AND rely on it being a valid UUID - if the user sends a string that is not a valid UUID it will cause an error.

I can safely replace the ids with other strings. Or to be more exact with other unique and valid UUIDs. The clients wont break if I change the IDs but they too will rely on them being UUIDs.

So should I use custom type or not?

[vladar]

Thanks! Released in v0.11.5

[vladar]

I wouldn't use separate UUID type in your case.

[enumag]

Alright, thank you!

[theofidry]

is it expected to have tabs here?

[theofidry]

Cheers 👍

[enumag]

@theofidry Damn. No of course not. I'm using tabs in my project so IDE automatically used them here... I'll fix it in another PR.

[enumag]

#211