facebook / relay

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

RefetchContainer's refetch refetchVariables param not giving previous variables. #1984

Closed sgwilym closed 2 years ago

sgwilym commented 7 years ago

In a refetchContainer, when I do the following:

this.props.relay.refetch(prevVars => {
  return {
    ...prevVars,
    count: 10,
    query: value,
  };
}, null);

prevVars is always an empty object. In my case, it's a refetchContainer wrapping a paginationContainer. I don't know if this is being caused by using graphql.experimental, but seeing as the refetchContainer seems to require its usage I'm at my wit's end on how to implement something that was trivial to implement with Relay Classic (a paginating list with many arguments on the connection).

pranaygp commented 7 years ago

@sgwilym Have you tried using the PaginationContainer https://facebook.github.io/relay/docs/pagination-container.html ?

( not that it fixes your issue, but should still be useful 😄 )

sgwilym commented 7 years ago

@pranaygp This is an example that I grabbed from the Relay docs, and not relevant to the problem.

alexhawkins commented 7 years ago

I ran into your exact issue and ended up getting prevVariables from this.context.relay.variables. I have refetch containers working throughout a massive app that I converted from Relay Classic to Relay Modern. The classic version is running here: https://beta.onehopewine.com/shop/wine/wine. Modern version should be up in a couple weeks. I use refetch containers for both pagination and filtering. Everything works flawlessly. See below.

  handleFilterChange = ({
    sortBy,
    filterPrice,
    filterByType,
    filterByCause,
    filterByVarietal,
    filterByGiftCategories,
  }) => {
    const { relay, params } = this.props;
    const { variables } = this.context.relay;

    const category = variables.category || params.category;
    const count = SIZE_PER_PAGE;
    const { isLoadingProducts } = this.state.data.toJS();

    if (!isLoadingProducts) {
      this.handleLoading();

      const refetchVariables = fragmentVariables => {
        return {
          count,
          sortBy,
          category,
          filterPrice,
          filterByType,
          filterByCause,
          filterByVarietal,
          filterByGiftCategories,
        };
      };

      relay.refetch(refetchVariables, null, () => this.handleLoading());
    }
  };

  loadNextPage = () => {
    const {
      relay,
      handleShowFooter,
      viewer: { user: { products } },
      params,
    } = this.props;
    const { variables } = this.context.relay;
    if (!products || !products.pageInfo.hasNextPage) {
      handleShowFooter(true);
      return;
    }
    this.handleScrolling();
    const category = variables.category || params.category;
    const sortBy = variables.sortBy || 'lowPrice';
    const currentTotal = variables.count || SIZE_PER_PAGE;
    const count = currentTotal + SIZE_PER_PAGE;
    const filterPrice = variables.filterPrice || ['any'];
    const filterByCause = variables.filterByCause || ['any'];
    const filterByType = variables.filterByType || ['any'];
    const filterByVarietal = variables.filterByVarietal || ['any'];
    const filterByGiftCategories = variables.filterByGiftCategories || ['any'];

    const refetchVariables = fragmentVariables => {
      return {
        count,
        category,
        sortBy,
        filterPrice,
        filterByType,
        filterByCause,
        filterByVarietal,
        filterByGiftCategories,
      };
    };

    relay.refetch(refetchVariables, null, () => this.handleScrolling());
  };

To get variables from this.context.relay, you're prob going to need to add this:

const contextTypes = {
  router: PropTypes.object.isRequired,
  relay: PropTypes.shape({
    variables: PropTypes.shape({
      count: PropTypes.number,
      category: PropTypes.string,
    }).isRequired,
  }).isRequired,
};

My refetch container looks like this:

export default createRefetchContainer(
  ProductsCatalog,
  {
    viewer: graphql.experimental`
      fragment ProductsCatalog_viewer on Viewer
        @argumentDefinitions(
          count: { type: "Int", defaultValue: 8 }
          category: { type: "String", defaultValue: "any" }
          sortBy: { type: "String", defaultValue: "lowPrice" }
          filterPrice: { type: "[String]", defaultValue: ["any"] }
          filterByCause: { type: "[String]", defaultValue: ["any"] }
          filterByType: { type: "[String]", defaultValue: ["any"] }
          filterByVarietal: { type: "[String]", defaultValue: ["any"] }
          filterByGiftCategories: { type: "[String]", defaultValue: ["any"] }
        ) {
        user {
          ...AuthDialog_user
          id
          role
          products(
            first: $count
            category: $category
            sortBy: $sortBy
            filterPrice: $filterPrice
            filterByCause: $filterByCause
            filterByType: $filterByType
            filterByVarietal: $filterByVarietal
            filterByGiftCategories: $filterByGiftCategories
          ) @connection(key: "ProductsCatalog_products") {
            edges {
              cursor
              node {
                id
                productId
                name
                price
                priceSale
                cartQty
                inStockOn
                wishListQty
                inventoryClass
                inventorySKU
                reviewAverage
                productURLslug
                isWishListFavorite
                images {
                  productCatalog
                }
                content {
                  impactSmall
                }
                ...ProductListItem_product
              }
            }
            pageInfo {
              hasNextPage
              hasPreviousPage
              startCursor
              endCursor
            }
          }
          cart {
            id
            taxRate
            taxTotal
            totalNumberOfItems
            subTotalPriceOfItems
            promoCodeSavings
            subTotalPriceOfItemsWithPromoCodes
            entries(first: 1000) {
              edges {
                node {
                  id
                  product {
                    id
                    productId
                    price
                    priceSale
                    cartQty
                    wishListQty
                    isWishListFavorite
                  }
                  quantity
                }
              }
            }
          }
          wishList {
            id
            name
            isDefault
            wishListId
            entries(first: 100) @connection(key: "WishList_entries") {
              edges {
                node {
                  id
                  product {
                    id
                    productId
                    price
                    priceSale
                    cartQty
                    wishListQty
                    isWishListFavorite
                    inventorySKU
                  }
                  quantity
                }
              }
            }
          }
        }
      }
    `,
  },
  graphql.experimental`
    query ProductsCatalogRefetchQuery(
      $count: Int
      $category: String
      $sortBy: String
      $filterPrice: [String]
      $filterByCause: [String]
      $filterByType: [String]
      $filterByVarietal: [String]
      $filterByGiftCategories: [String]
    ) {
      viewer {
        ...ProductsCatalog_viewer
          @arguments(
            count: $count
            category: $category
            sortBy: $sortBy
            filterPrice: $filterPrice
            filterByCause: $filterByCause
            filterByType: $filterByType
            filterByVarietal: $filterByVarietal
            filterByGiftCategories: $filterByGiftCategories
          )
      }
    }
  `,
);

Note that I'm using found and found-relay for routing and am using prepareVariables. For example:


<Route
      path="shop/wine/:category"
      getQuery={() => require('../queries/ProductCatalog').default}
      getComponent={lazyLoadProductCatalog}
      prepareVariables={prepareProductCatalogParams}
      render={renderComponent}
    />```
alexhawkins commented 7 years ago

It was definitely worth converting from classic to modern ( the speed gains are ridiculous and classic was already fast ). However, the migration path is certainly not as easy as certain folks are making it out to be. Documentation is sparse so you pretty much have to read the source code and figure this stuff out on your own. I haven't been able to figure out caching with Relay Modern as of yet...a feature that was just baked into classic it would seem. Modern is still way faster ( don't really need caching to be honest ) and makes a lot more sense to me. But I would argue that it's actually more complicated than Relay Classic. At least at this stage in its development. There really needs to be better documentation or people will just end up using Apollo. Hopefully, my post above helps someone.

scotmatson commented 7 years ago

@alexhawkins Right there with you. I've spent the last few days trying to come up with a offset-based pagination solution and the createPaginationContainer just isn't supportive of that feature. I get that Relay is opinionated and targeting Facebook's infrastructure but there are just some very basic use cases that should be baked in. I thought I could just use the createRefetchContainer and build this myself but once again I'm running into problems (which is how I ended up here). Relay has such great potential but if everything continues to feel like a Hack just to implement some very basic and common cases I may just end up having to settle in with Apollo.

kokolegorille commented 7 years ago

I used this trick to make it work...

  _loadMore = () => {
    // // Increments the number of records being rendered by 10.
    // const refetchVariables = fragmentVariables => {
    //   console.log(`first ${fragmentVariables.first}`);
    //
    //   return {
    //     first: fragmentVariables.first + 10
    //   };
    // };
    //
    // this.props.relay.refetch(refetchVariables);

    const {store} = this.props;
    const refetchVariables = {first: store.players.edges.length + 50}
    this.props.relay.refetch(refetchVariables);
  }

When using the commented code, I received a correct value on fragmentVariables, but variables was not set when using a function as parameter to refetch. So first fetch was ok, but not the second.

The dirty trick was to calculate the size of my cursor (store.players.length), then add 50 and pass it to refetch using an object parameter.

I hope this can be fixed soon...

kokolegorille commented 7 years ago

BTW in the source code of the refetch container, You find...

    _getFragmentVariables(): Variables {
      const {
        getVariablesFromObject,
      } = this.context.relay.environment.unstable_internal;
      return getVariablesFromObject(
        this.context.relay.variables,
        fragments,
        this.props,
      );
}

But looking at the relay props, I just have environment and refetch, but not variables

PS: I'm using relay 1.3.0

sibelius commented 7 years ago

I think @kassens fixed this here https://github.com/facebook/relay/commit/1ce348a5bc472a58607279c08e2236b8459b5877

kokolegorille commented 7 years ago

Thank You for the info, I will give it a try and update to this version.

It is complicate to make it work, as they are not a lot of documentation.

2017-09-12 17:09 GMT+02:00 Sibelius Seraphini notifications@github.com:

I think @kassens https://github.com/kassens fixed this here 1ce348a https://github.com/facebook/relay/commit/1ce348a5bc472a58607279c08e2236b8459b5877

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/facebook/relay/issues/1984#issuecomment-328860320, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBqhy9hvyxlPdFB6wbVF6UMhEay_Uriks5sho14gaJpZM4OfOkH .

sibelius commented 7 years ago

@kassens I've tried on master and this is not working well on compat mode

If I defined a local variables using @argumentDefinitions and does not provide a defaultValue

this variable won't appear on this.context.relay.variables, neither in the graphql query sent to server

sibelius commented 7 years ago

@sgwilym is this still happening?

toxsick commented 7 years ago

This is still happening in our project with 1.4.1 and compat mode

bchenSyd commented 7 years ago

solved by PR https://github.com/facebook/relay/pull/1868

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

sibelius commented 2 years ago

Migrate to relay hooks, if the issue still valid, open another issue