Skip to content
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

Fix: partial rendering #236

Merged
merged 3 commits into from
Feb 16, 2023
Merged

Conversation

Lalitha-Iyer
Copy link
Contributor

@Lalitha-Iyer Lalitha-Iyer commented Feb 9, 2023

I noticed partial data rendering was not working .While debugging I noticed we weren't passing the renderPolicy option to Fetchresolver.fetch method https://github.com/relay-tools/relay-hooks/blob/master/src/FetchResolver.ts#L120

@Lalitha-Iyer
Copy link
Contributor Author

@morrys Let me know if I should create an issue first to describe the problem.

Copy link
Member

@morrys morrys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lalitha-Iyer thanks for finding this bug 💯

No you don't need an issue 👍

Just put this test case after: https://github.com/relay-tools/relay-hooks/blob/master/__tests__/useLazyLoadQueryNode-test.tsx#L567

describe('partial rendering', () => {
        it('does not suspend at the root if query does not have direct data dependencies', () => {
            const gqlFragment = graphql`
                fragment useLazyLoadQueryNodeTestRootFragment on Query {
                    node(id: $id) {
                        id
                        name
                    }
                }
            `;
            const gqlOnlyFragmentsQuery = graphql`
                query useLazyLoadQueryNodeTestOnlyFragmentsQuery($id: ID) {
                    ...useLazyLoadQueryNodeTestRootFragment
                }
            `;
            const onlyFragsQuery = createOperationDescriptor(gqlOnlyFragmentsQuery, variables);

            function FragmentComponent(props: { query: any }) {
                const data: any = useSuspenseFragment(gqlFragment, props.query);
                renderFn(data);
                return null;
            }

            const Renderer = (props: { variables: { id: string } }) => {
                const { data } = useLazyLoadQuery<any>(gqlOnlyFragmentsQuery, props.variables, {
                    //fetchObservable: __internal.fetchQuery(environment, _query),
                    fetchPolicy: 'store-or-network',
                    UNSTABLE_renderPolicy: 'partial',
                });
                return (
                    <React.Suspense fallback="Fallback around fragment">
                        <FragmentComponent query={data} />
                    </React.Suspense>
                );
            };

            const instance = render(environment, <Renderer variables={variables} />);

            // Assert that we suspended at the fragment level and not at the root
            expect(instance.toJSON()).toEqual('Fallback around fragment');
            expectToHaveFetched(environment, onlyFragsQuery);
            expect(renderFn).not.toBeCalled();
            // $FlowFixMe[method-unbinding] added when improving typing for this parameters
            expect(environment.retain).toHaveBeenCalledTimes(1);

            environment.mock.resolve(gqlOnlyFragmentsQuery, {
                data: {
                    node: {
                        __typename: 'User',
                        id: variables.id,
                        name: 'Alice',
                    },
                },
            });

            // $FlowFixMe[incompatible-call] Error found while enabling LTI on this file
            expectToBeRendered(renderFn, {
                node: {
                    id: variables.id,
                    name: 'Alice',
                },
            });
        });
    });

And change this import: https://github.com/relay-tools/relay-hooks/blob/master/__tests__/useLazyLoadQueryNode-test.tsx#L20

import {
    useLazyLoadQuery as useLazyLoadQueryNode,
    RelayEnvironmentProvider,
    useLazyLoadQuery,
    useSuspenseFragment,
} from '../src';

This way you will see that without your modification the test case fails :)

Test is taken from relay official with simple adaptations for relay-hooks

@Lalitha-Iyer
Copy link
Contributor Author

Lalitha-Iyer commented Feb 15, 2023

Thanks for that great suggestion, I wasn't sure where to add this test case.
I tested it locally and the new test passed. I also confirmed that the test failed on the "master" branch.

@Lalitha-Iyer
Copy link
Contributor Author

Tangentially could you help me understand, why this option has an UNSTABLE prefix?
I am considering using this in production code, is it safe to assume this option will not be deprecated in future versions?

@morrys
Copy link
Member

morrys commented Feb 15, 2023

relay-hooks is the open source version of react-relay with support for hooks that don't use Suspense.

The goal is to follow as much as possible the evolution of react-relay but above all of relay-runtime.

In this case, for relay that property is still considered unstable https://github.com/facebook/relay/blob/main/packages/react-relay/relay-hooks/useLazyLoadQuery.js#L40

@morrys morrys merged commit 614bfcb into relay-tools:master Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants