search

facebook / relay

Relay is a JavaScript framework for building data-driven React applications.
https://relay.dev
MIT License
18.38k stars 1.82k forks source link

Validate edgeTypeName argument points to an existing type #4621

Closed tobias-tengler closed 3 months ago

tobias-tengler commented 7 months ago

Currently you can input any string for the edgeTypeName argument on the @prependNode and @appendNode directives.

This change enforces that the value you supply points to an existing object type within the schema.

captbaritone commented 7 months ago

So good! Thank you!

facebook-github-bot commented 7 months ago

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

captbaritone commented 6 months ago

Still working on landing this. It looks like it's triggering a number of errors internally. Trying to figure out if this is because we're catching real errors (and if so, how we can resolve them), or if there's something else going on here.

tobias-tengler commented 5 months ago

Is this still blocked internally?

captbaritone commented 5 months ago

Sorry, haven't been able to revisit. Adding to my todo list for next week.

captbaritone commented 5 months ago

So we have many infractions in our codebaese today, and I don't see an obvious way to "fix" these issues at scale. Maybe we need a mechanism to selectively ignore this warning for existing infractions? Perhaps we could leverage our feature flag infra here?

Also, would be cool to suggest types with similar names in this error, I think we have examples of that elsewhere in the codebase.

tobias-tengler commented 5 months ago

@captbaritone Added object type suggestions and a feature flag to disable the validation :)

facebook-github-bot commented 4 months ago

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

tobias-tengler commented 4 months ago

@captbaritone I fixed the conflicts :)

facebook-github-bot commented 3 months ago

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot commented 3 months ago

@captbaritone merged this pull request in facebook/relay@53b24361bb4d761411474508bd08f61fedb6b1cc.

facebook-github-bot commented 3 months ago

This pull request has been reverted by 43bd9bedb2b05920e61c093c3311f68bfd910fd8.

tobias-tengler commented 3 months ago

@captbaritone why was this reverted? Internal reasons?

captbaritone commented 3 months ago

@tobias-tengler Yeah, internal reasons. Just some internal miscommunication with getting the feature flag set. I have an internal diff open to re-land.