Relay: 7.0.0 bug: required argument 'ABC' is missing on 'XYZ'

[mrtnzlml]

Hi, I am trying to upgrade to 7.0.0 but I am getting many errors from the compiler (sample):

- Required argument 'windowDuration: String!' is missing on 'historySpending' in 'FinanceAccountDailySpendingTable_account_3Cu9cz'.

  Source: components/dailySpending/FinanceAccountDailySpendingTable.js (4:7)
  3:       @argumentDefinitions(interval: { type: "FinanceAccountDateTimeInterval!" }) {
  4:       historySpending(interval: $interval) {
           ^
  5:         edges {

  Source: components/dailySpending/FinanceAccountDailySpendingTable.js (2:5)
  1: 
  2:     fragment FinanceAccountDailySpendingTable_account on FinanceAccount
         ^
  3:       @argumentDefinitions(interval: { type: "FinanceAccountDateTimeInterval!" }) {

- Required argument 'currencyType: FinanceAccountCurrencyType!' is missing on 'topUpValue' in 'FinanceAccountTopUpsTable_topUps'.

  Source: components/account/accountTopUps/FinanceAccountTopUpsTable.js (9:11)
   8:           status
   9:           topUpValue {
                ^
  10:             ...Money_money

  Source: components/account/accountTopUps/FinanceAccountTopUpsTable.js (2:5)
  1: 
  2:     fragment FinanceAccountTopUpsTable_topUps on FinanceAccountTopUpConnection {
         ^
  3:       edges {

I believe this is a bug because all of these arguments are defined as required in the schema BUT they all have a default value like so:

type FinanceAccountTopUp implements Node {
  # ...
  topUpValue(currencyType: FinanceAccountCurrencyType! = BALANCE): Money
}

Therefore these arguments should not be required. Thanks for having a look guys! :)

[mrtnzlml]

It reminds me this bug in GraphQL.js I reported earlier but not sure if it's related since this dependency is being removed: https://github.com/graphql/graphql-js/issues/2190

[alunyov]

@mrtnzlml thanks for reporting! I'll take a look. It's probably this line: https://github.com/facebook/relay/blob/16e0caf45cecf8501fa511c1fea6ea5572576c5d/packages/relay-compiler/transforms/ValidateRequiredArgumentsTransform.js#L113

[alunyov]

Oh, while we on this topic @mrtnzlml. In the places where you're spreading FinanceAccountDailySpendingTable do you provide this $interval with @arguments?

[mrtnzlml]

@alunyov Yes, that is correct. This is the fragment with spread:

fragment FinanceAccountHistoryData_account on FinanceAccount
  @argumentDefinitions(interval: { type: "FinanceAccountDateTimeInterval!" }) {
  type
  ...FinanceAccountHistoryBalancesChart_account @arguments(interval: $interval)
  ...FinanceAccountDailySpendingTable_account @arguments(interval: $interval)
}

And here is the problematic fragment:

fragment FinanceAccountDailySpendingTable_account on FinanceAccount
  @argumentDefinitions(interval: { type: "FinanceAccountDateTimeInterval!" }) {
  historySpending(interval: $interval) {
    edges {
      node {
        windowInterval {
          start
        }
        ...FinanceAccountDailySpendingRow_historySpending
      }
    }
  }
}

The is how is the field defined:

type FinanceAccount implements Node {
  historySpending(
    after: String
    before: String
    first: Int
    interval: FinanceAccountDateTimeInterval!
    last: Int

    """
    Duration uses syntax defined by ISO 8601. It is used to select two balances between which spending is calculated.
    """
    windowDuration: String! = "P1D"
  ): FinanceAccountHistorySpendingConnection
}

Are we doing it wrong? The second error example seems to be much simpler - just a fragment without any args where we use topUpValue field also without any args (they are optional).

[mrtnzlml]

Hi @alunyov! I tried to fix this issue here: https://github.com/facebook/relay/pull/2903

Could you please check it and consider releasing this fix in a patch version? It's blocking the upgrade to version 7.0.0 for us. Thank you very much! :)