Skip to content

Conversation

@simoneb
Copy link
Contributor

@simoneb simoneb commented Jan 28, 2022

Closes #30

Note that this is the fastest solution to the issue I came up with but I'm not 100% sure that this covers all scenarios or that it doesn't break anything.

The change is minimal but has cascading consequences on many assertions in tests because most types and fields are nullable, therefore will see their json-schema type change from 'whatever' to ['whatever', 'null'].

The scenario raised by the OP is covered and works fine.

Copy link
Contributor

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, this is quite a serious bug indeed.

@mcollina mcollina requested a review from jonnydgreen January 28, 2022 17:42
@mcollina
Copy link
Contributor

@jonnydgreen wdyt? what semversiness this should be?

I'm tempted for a patch and a backport to previous lines too, but it's also a big change.

Copy link
Collaborator

@jonnydgreen jonnydgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks for sorting this!

@jonnydgreen
Copy link
Collaborator

jonnydgreen commented Jan 28, 2022

I'm tempted for a patch and a backport to previous lines too, but it's also a big change.

Yeah, I agree with the patch and backport approach - especially as it's fixing spec compliance. Thinking the following?

  • 2.0.0 => 2.0.1
  • 1.0.0 => 1.0.1

@mcollina
Copy link
Contributor

That's the idea.

@mcollina mcollina merged commit 3320abc into mercurius-js:main Jan 31, 2022
@jonnydgreen
Copy link
Collaborator

Apologies, I missed this message as I was afk all weekend with the intention of doing this first thing Monday - thank you for releasing these fixes earlier! :)

@mcollina
Copy link
Contributor

no worries, we needed these out asap

@paolochiodi
Copy link

Just tested this in my code and works as expected.
Thanks @simoneb and rest of the team.

@simoneb simoneb deleted the feat/30 branch February 3, 2022 09:26
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.

mercurius-validation doesn't support nullable type correctly

4 participants