-
Notifications
You must be signed in to change notification settings - Fork 129
Fix #489: Json Validator #513
base: master
Are you sure you want to change the base?
Conversation
@@ -59,6 +59,14 @@ export class Validator { | |||
) | |||
) | |||
}, | |||
Json: (str) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm getting an object in str so JSON.parse will always failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Json: (str) => {
if(typeof str === 'string') {
try {
JSON.parse(str);
} catch (e) {
return false;
}
return true;
}
return true;
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you get an object in str, you should get a string on Json format.
If you have an object, the test should fail...
Did I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GaelGRIFFON Your code is assuming that the input to the Json
function will be a string, but actually, it's an object. That's because the exporter already exports it as a javascript object.
The exporter serializes everything as a json file and Json fields "could" be serialized as a string in Json format, but they're not. They're serialized as a plain old javascript object. Or null. Or undefined. Or 0. Or literally any serializable js type. We already know that it's valid json because the original 000001.json
file was successfully parsed. So at the time the import validator is running, this could be any type and it would be "valid json". Give that, here's my proposed solution:
Json: () => true,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK got it!
Thx you
@marktani @timsuchanek Can one of you please merge this? It's breaking a critical production feature for lots of people. |
Because already parsed
@dpetrick Since this is a critical bug, I agree this should be merged even if the graphcool-framework isn't a priority project for prisma. |
any workarounds ? |
Fix #489: Json Validator