- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 74
ConstraintString type appeared in graphql schema on the client #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
No, certainly not by design. Possibly a bug within |
I have the same problem, and I filled an issue within Edit: I'm having this issue when trying to make schema stitching on underlying schema having directive constraints |
UPD: ignore the below. The directive stopped working. :) I found a workaround! Omg, took a while. Add this to your # Current implementation on graphql-import does not support directives declared outside of the scehma (in node.js code).
# We have to redeclare each directive and type for now.
directive @constraint(
# String constraints
minLength: Int
maxLength: Int
startsWith: String
endsWith: String
notContains: String
pattern: String
format: String
# Number constraints
min: Int
max: Int
exclusiveMin: Int
exclusiveMax: Int
multipleOf: Int
) on INPUT_FIELD_DEFINITION
scalar ConstraintString
scalar ConstraintNumber |
@confuser hey James, Quick question. In this article Any reason you went by creating the new |
@koresar The recommendation seemed to be using custom scalar types to achieve this, rather than overriding the resolver function https://www.apollographql.com/docs/graphql-tools/schema-directives.html#Enforcing-value-restrictions I'm certainly open to switching to a resolver function instead if it eases pains people are facing. |
Frankly, I tried this: class MyConstraintDirective extends SchemaDirectiveVisitor {
static getDirectiveDeclaration(directiveName) {
return new GraphQLDirective({
name: directiveName,
locations: [DirectiveLocation.INPUT_FIELD_DEFINITION]
});
}
visitInputFieldDefinition(field) {
field.resolve = async function() {
throw new Error("not valid");
};
}
} But for some reason the So, not sure what to do frankly. |
I debugged a little. TL;DR: The
Here is a screenshot. I made it in node.js debugger one step before exiting the @confuser where should I rant about it next? In apollographql/graphql-tools ? :) |
I solved the problem. The
Turns out Prisma and Apollo packages are not fully compatible. Now I'm using Apollo Stack v2 (currently RC). The documentation does not match the actual API yet, but I figured all out. |
@koresar Interesting, so the new |
Sorry, should have mentioned. There were actually a lot of changes. I had to rewrite schema stitching.
It did solve all the problems I had. The high level solution: do not use Prisma and Apollo packages in the same project. They are often incompatible. |
Ok. I was wrong. Again. No, I did not fix all the problems I have. Even Apollo Stack v2 didn't help. Currently:
If I follow the README.md instructions then I get this error in the GraphQL Playground console: If I add I believe that official Apollo docs are wrong: https://www.apollographql.com/docs/apollo-server/v2/features/creating-directives.html#Enforcing-value-restrictions |
I've reported the issue here: apollographql/apollo-server#1303 @confuser It is so frustrating. Do you know any other ways the directive can be implemented? I am ready to do it myself. Just need an idea. |
@koresar Were you not able to overwrite the resolver function at all? If not, there is one other way, but it was deprecated in favour of the current approach, so I'd be hesitant of using it... directiveResolvers |
I tried directiveResolvers. Exactly the same issue - the directive resolver function is never executed. I tried all kind of approaches (including reimplementing the directive myself). Nothing works. Basically, the directive is in a non-working state atm. |
Strange... I have a project that uses directiveResolvers as a form of ACL and it works. I'll find some time and investigate what's happening. |
Which graphql-tools version it uses? |
I'm currently having exactly the same issue. Super frustrating. Haven't had any success yet. Switched to Apollo server 2 from Yoga I had an idea of perhaps doing MakeExecutableSchema twice and then combining them, getting the generated validated scalars from the first one into the other one |
Woot! Found a workaround. First make an executable schema for the scalar schema, and then merge that with the rest. It's hacky and I don't like it, but at least I can move on now.
|
Hey @FrankSandqvist How did you attach these two schemas to Playground and to the API? |
I pass in the merged schema finalSchema, also re-pass the resolvers. It's hacky, again, but it works for now.
|
Of course this still doesn't solve the problem that the scalars are public. For example StringWithMaxLength50WithMinLength5ThatConformsToEMAIL |
Dear @FrankSandqvist People like you deserve monuments erected. (That's my way of saying "thank you, it works"). |
Hahah, that's awesome! I've always wanted a thinking Shakespeare ape monument in my name 😄 |
Any solution or update on this? |
Unfortunately not, dependent on apollo |
I am also waiting for a solution to this 😄 |
Still having this issue as well. Is there a plan to fix this? |
@FrankSandqvist Could you share how your Scalar files that you read in look like? |
I ran across this issue trying to solve a similar issue in a custom directive I created. My situation was slightly different, but I came up with a solution that could work for this issue as well. It would require an update to the directive and I'm not sure how/if it would affect non-Apollo users. The issue is that a new type is being created in the directive schema visitor, but the type is not being added to the schema type map. For example, https://github.com/confuser/graphql-constraint-directive/blob/master/index.js#L54. If the type is not added to the schema type map, then the above mentioned error "Incomplete or Invalid schema..." will occur when Playground introspects the schema. The following could be a possible solution. This example could be simplified since the new type is known to be either ConstraintStringType or ConstraintNumberType, but I left it a bit more generic for anyone else who might run across this issue. Note, isNamedType and isWrappingType is imported from 'graphql'. A possible solution could be to modify the wrapType function:
|
Just checking in here; has anybody seen any movement from Apollo on a fix? Should I just consider implementing @FrankSandqvist 's workaround? Thanks! |
Hi there. Here is a fresh out of the press new (forked) constraint directive. It's using the idea suggested by @padenj |
@padenj code does not work if you have the input CreateUserInput {
login: String! @constraint(minLength: 1)
password: String! @constraint(minLength: 1)
} because of this if: if (isNamedType(type) && !typeMap[type.name]) {
typeMap[type.name] = type;
} Steps of directive resolution:
The Apparently scalar types in Apollo (and/or GraphQL) can have only one resolver for a scalar type. Theorically it makes sense, because any field of the same type must be serialized or parsed in the same way. If you don't add @padenj code, the directives will work but not the introspection, and we return to the main problem of this issue. |
@julianomqs creating a specific scalar type using the fieldName per example would be a solution. class StringConstraintType extends GraphQLScalarType {
constructor(fieldName, type, args) {
super({
name: `StringConstraint${fieldName.toUpperCase()}`,
serialize(value) {
value = type.serialize(value);
validate(fieldName, value, args);
return value;
},
parseValue(value) {
value = type.parseValue(value);
validate(fieldName, value, args);
return value;
},
parseLiteral(ast) {
const value = type.parseLiteral(ast);
validate(fieldName, value, args);
return value;
}
});
}
} |
This works for now! Thanks.
|
ConstraintString leaked through introspection. Is it by design?
This makes GraphiQL unhappy.
ConstraintString can be revealed by the following query:
The text was updated successfully, but these errors were encountered: