Skip to content

Conversation

@spawnia
Copy link
Collaborator

@spawnia spawnia commented Aug 2, 2021

No description provided.

@codecov
Copy link

codecov bot commented Aug 2, 2021

Codecov Report

Merging #906 (05c4399) into master (bbb6e60) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 05c4399 differs from pull request most recent head 58f21b8. Consider uploading reports for the commit 58f21b8 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #906      +/-   ##
==========================================
- Coverage   94.11%   94.11%   -0.01%     
==========================================
  Files         117      117              
  Lines        9683     9679       -4     
==========================================
- Hits         9113     9109       -4     
  Misses        570      570              
Impacted Files Coverage Δ
src/Type/Definition/BooleanType.php 100.00% <ø> (ø)
src/Type/Definition/EnumType.php 81.57% <ø> (ø)
src/Type/Definition/FloatType.php 100.00% <ø> (ø)
src/Type/Definition/IDType.php 100.00% <ø> (ø)
src/Type/Definition/InputObjectType.php 92.30% <ø> (ø)
src/Type/Definition/IntType.php 100.00% <ø> (ø)
src/Type/Definition/InterfaceType.php 97.87% <ø> (ø)
src/Type/Definition/ObjectType.php 98.11% <ø> (ø)
src/Type/Definition/StringType.php 100.00% <ø> (ø)
src/Type/Definition/UnionType.php 95.55% <ø> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bbb6e60...58f21b8. Read the comment docs.

@spawnia spawnia merged commit d8ccdfe into master Aug 2, 2021
@spawnia spawnia deleted the native-types-for-types branch August 2, 2021 09:29
@sebastienbarre
Copy link

sebastienbarre commented Aug 16, 2021

I love native types, but this is possibly going to trigger fatal errors and break a lot of people's code.

For example:

class SearchTermType extends ScalarType
{
  /**
   * Scalar type description.
   */
  public $description = 'The `SearchTerm` scalar type represents textual data [blahblah].';
...

It is now breaking:

Fatal error: Type of SearchTermType::$description must be ?string (as in class GraphQL\Type\Definition\Type) inSearchTermType.php on line 0

This is also breaking external dependencies that provide custom scalars. For example, mll-lab/graphql-php-scalars: A collection of custom scalar types for usage with https://github.com/webonyx/graphql-php:

Fatal error: Type of MLL\GraphQLScalars\Email::$description must be ?string (as in class GraphQL\Type\Definition\Type) in Vendor/mll-lab/graphql-php-scalars/src/Email.php on line 10

and these I can't fix without manually patching.
UPDATE: oops I just realized you are the maintainer of graphql-php-scalars, never mind :) Thanks for the good work.

This is for a major release I assume, version 15?

@spawnia
Copy link
Collaborator Author

spawnia commented Aug 17, 2021

I am maintaining v15 compatible branches of my packages depending on this library:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants