Highlight Issue: Todo example with Relay v15.0.0 and @required field

[eduardolundgren]

Changes

• Updated the Todo demo to demonstrate with Relay v15.0.0
• Added the @required field which was experimental in v12.0.0
• Contrasted with the official Relay repo's Todo demo to show that @required is not functioning as expected in v15.0.0. Exceptions should have been thrown, but aren't (see attached screenshot for a comparison).
• This PR is to bring attention to this observed issue with the @required field at relay-hooks with Relay v15.0.0.

Question

In addition to the above observations regarding the @required field with Relay v15.0.0, I'd like to ask if there are any plans or intentions to fully support Relay v15.0.0 in the near future? It would be helpful to know for those of us who are considering upgrading or building upon this version. Thank you.

[morrys]

Hi @eduardolundgren, thanks for the PR 👍 I made some changes and drafted the PR.

• Transformed the non-mandatory text field on the server side. Having the required field and returns null as a value, causes an error in the network response https://github.com/relay-tools/relay-hooks/pull/251/commits/e9f024ad23fbd5330d6bb0b2c5ab468c0c43e2e0
• Restored the use of the fragment that used the @required directive. Without this change the directive was ignored as it was not used https://github.com/relay-tools/relay-hooks/pull/251/commits/835a3faa898e66c21f35d33a3f7adb4f7b127d19
• WIP solution implemented, todos: https://github.com/relay-tools/relay-hooks/pull/251/commits/44e7c17def252439b0b1f7b8bbf93d3ae4e1ad17
  • The "handlePotentialSnapshotErrors" method added in the relay v13.2.0 version while relay-hooks has peerDependencies "^13.0.2", possible breaking change
  • In relay "snapshot.missingData" is checked before call handlePotentialSnapshotErrors while here in relay-hooks we check "snapshot.missingRequiredFields". We need to investigate possible different behavior between concurrent and non-concurrent mode.

As far as relay v15 support is concerned, the idea is to support it entirely with particular interest in features that are both concurrent and non-concurrent.

Being a project that I carry out in my free time, some of them may involuntarily escape me but I am always available to integrate them or evaluate PRs that do so.

[eduardolundgren]

All changes look good to me.

Given these @required enhancements, would it be possible to kindly consider releasing a new version with these changes?

Thanks for your hard work on this project.

[morrys]

Hi @eduardolundgren, this PR has been released https://github.com/relay-tools/relay-hooks/releases/tag/v8.0.1

let me know if there are any problems :)

[eduardolundgren]

Thank you, @morrys. I will test version 8.0.1 in the coming days and inform you of any issues.

[eduardolundgren]

Hi @morrys, we tested it on the product codebase, and it works as expected. We're investigating a use case that might be atypical: we want to invalidate the cache for the query that resulted in an exception from @required. I've noticed that the error thrown originates from inside a RelayObservable and doesn't bubble up. We'll continue our investigation, but I wanted to share this with you in case you have any insights on its feasibility. Having access to the component or query it came from, this could be achieved either by interacting with the relay store or by accessing the component instance and incrementing the fetchKey. Thanks.

[morrys]

Hi @eduardolundgren, have you tried opening an issue in relay? This seems like a problem that could be fixed in relay-runtime

[eduardolundgren]

@morrys I've noticed that the exception isn't thrown when @required is inside a fragment. Is this expected?

[morrys]

@eduardolundgren In the example project in this PR the error is thrown by the fragment.