Remove/replace isResourceClass
#3420
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now I've encoded the expectations more directly. That it is a GQL interface & to avoid the intermediate intersected classes.
With the `Props` list no longer being a requirement, the only runtime requirement we have is the value having a `prototype`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With
Props/SecuredPropson the chopping block, we don't really a have a way at runtime to check if a class is a resource.But really what is a "resource"? It has some kind of alignment with GQL's
Resourceinterface, but it is kinda fuzzy. It really could be any class model at this point. I think this is ok. We need to facilitate some inner workings that we don't want exposed to the GraphQL schema. Models need to be registered for dynamic lookup, be used in policies, but maybe there are situations we don't want to officially declare them, with@RegisterResource.Some current examples are
Pinnable,ChangesetAware.I'm not sure. I'm not sure I believe the above. Fuzziness just leads to confusion.
The alternative to this, is to just check if the resource has been registered, and force that for consistency.
Maybe that's the next step.
This at least moves us away from depending on the
Propsarray.Aside: 80a9510 is in that other PR, but it contributes to this notion here.