Consider turning validation of fields with dunderscore prefix back into warning.

[alloy]

Based on some old discussion (not sure where anymore so can’t provide links) we decided to use __id for global object IDs in our schema.

Recently, an initial change to the name validation started throwing errors for fields starting with a dunderscore. This was then turned into a warning so legacy schemas wouldn’t immediately fail to load. But then this recent change turned the warnings into hard errors again.

While I completely understand that this name is not in line with the spec, graphql-js is < v1 which according to semver means anything can change, and in principle throwing an error makes sense; the problem is that I’d rather not have to make very hard breaking changes to our schema (and break current iOS app versions) until either:

• we are going to make sweeping changes anyways (we’re currently experimenting with stitching schemas from backend services, which could lead to a great time to introduce a v2 of our schema)
• there are technical reasons for graphql-js to no longer being able to support fields with dunderscore prefixes

I think the previous logged warning was big enough that people wouldn’t adopt new fields in their schema that use a dunderscore prefix.

PS: I’m mostly making this case for others in the community, I have no problem simply disabling the check in our apps.

[leebyron]

Thanks for highlighting this - I agree there should be an escape hatch for this.

For some context on that discussion you're referring to, __id is specifically the case that is problematic - there have been discussions about using it in the spec as a way to access internal global identifiers but allowing user-defined schema to use the field name makes it impossible to introduce such a change. We decided that this probably wouldn't be the first time we realized the mistake, so put a blanket restriction on user-defined schema fields and types from using dunderscore prefixes. A warning was put in place a few months ago, this recent change upgraded the warnings to explicit validation errors.

I'll keep this issue open to track an escape hatch for the error. We'll get something in place before the next release

[alloy]

Makes total sense 👍

I’d be happy to make a PR for this. Would you want it to work the same way as previously? (Upside is that the env var that people may already have set would continue to work.)

[leebyron]

I've got one in progress I'll put up in a couple minutes and ping you for your thoughts