Fix incorrect type generation with @required on pluarl fragments

Reviewed By: kassens

Differential Revision: D35344295

fbshipit-source-id: 031a08e98c9d8179f33901d86e54c37dec5a02ba

compiler/crates/relay-typegen/src/lib.rs

@@ -440,25 +440,24 @@ impl<'a> TypeGenerator<'a> {
 
         let unmasked = RelayDirective::is_unmasked_fragment_definition(fragment_definition);
 
-        let base_type = self.selections_to_babel(
+        let mut type_ = self.selections_to_babel(
             selections.into_iter(),
             unmasked,
             if unmasked { None } else { Some(fragment_name) },
         );
-        let type_ = if is_plural_fragment {
-            AST::ReadOnlyArray(base_type.into())
-        } else {
-            base_type
-        };
 
-        let type_ = match fragment_definition
+        if fragment_definition
             .directives
             .named(*CHILDREN_CAN_BUBBLE_METADATA_KEY)
+            .is_some()
         {
-            Some(_) => AST::Nullable(type_.into()),
-            None => type_,
+            type_ = AST::Nullable(type_.into());
         };
 
+        if is_plural_fragment {
+            type_ = AST::ReadOnlyArray(type_.into())
+        }
+
         self.runtime_imports.fragment_reference = true;
         self.write_import_actor_change_point()?;
         self.write_fragment_imports()?;

compiler/crates/relay-typegen/tests/generate_flow/fixtures/required-bubbles-to-plural-fragment-root.expected

@@ -0,0 +1,32 @@
+==================================== INPUT ====================================
+fragment SomeFragment on User @relay(plural: true) {
+  name @required(action: LOG)
+}
+
+fragment SomeOtherFragment on User @relay(plural: true) {
+  name
+}
+==================================== OUTPUT ===================================
+import type { FragmentType } from "relay-runtime";
+declare export opaque type SomeFragment$fragmentType: FragmentType;
+export type SomeFragment$data = $ReadOnlyArray<?{|
+  +name: string,
+  +$fragmentType: SomeFragment$fragmentType,
+|}>;
+export type SomeFragment$key = $ReadOnlyArray<{
+  +$data?: SomeFragment$data,
+  +$fragmentSpreads: SomeFragment$fragmentType,
+  ...
+}>;
+-------------------------------------------------------------------------------
+import type { FragmentType } from "relay-runtime";
+declare export opaque type SomeOtherFragment$fragmentType: FragmentType;
+export type SomeOtherFragment$data = $ReadOnlyArray<{|
+  +name: ?string,
+  +$fragmentType: SomeOtherFragment$fragmentType,
+|}>;
+export type SomeOtherFragment$key = $ReadOnlyArray<{
+  +$data?: SomeOtherFragment$data,
+  +$fragmentSpreads: SomeOtherFragment$fragmentType,
+  ...
+}>;

compiler/crates/relay-typegen/tests/generate_flow/fixtures/required-bubbles-to-plural-fragment-root.graphql

@@ -0,0 +1,7 @@
+fragment SomeFragment on User @relay(plural: true) {
+  name @required(action: LOG)
+}
+
+fragment SomeOtherFragment on User @relay(plural: true) {
+  name
+}

compiler/crates/relay-typegen/tests/generate_flow_test.rs

@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<72f4bf167568e5c4a2a2218c8bc29ac6>>
+ * @generated SignedSource<<0d8c404a6c037950a24e7f8515f31878>>
  */
 
 mod generate_flow;
@@ -348,6 +348,13 @@ fn required_bubbles_to_non_null_plural_linked_field() {
     test_fixture(transform_fixture, "required-bubbles-to-non-null-plural-linked-field.graphql", "generate_flow/fixtures/required-bubbles-to-non-null-plural-linked-field.expected", input, expected);
 }
 
+#[test]
+fn required_bubbles_to_plural_fragment_root() {
+    let input = include_str!("generate_flow/fixtures/required-bubbles-to-plural-fragment-root.graphql");
+    let expected = include_str!("generate_flow/fixtures/required-bubbles-to-plural-fragment-root.expected");
+    test_fixture(transform_fixture, "required-bubbles-to-plural-fragment-root.graphql", "generate_flow/fixtures/required-bubbles-to-plural-fragment-root.expected", input, expected);
+}
+
 #[test]
 fn required_bubbles_to_query() {
     let input = include_str!("generate_flow/fixtures/required-bubbles-to-query.graphql");

packages/relay-runtime/store/__tests__/RelayReader-RequiredFields-test.js

@@ -16,7 +16,7 @@ const {
 } = require('../RelayModernOperationDescriptor');
 const {read} = require('../RelayReader');
 const RelayRecordSource = require('../RelayRecordSource');
-const {createReaderSelector} = require('relay-runtime');
+const {createReaderSelector, getPluralSelector} = require('relay-runtime');
 
 describe('RelayReader @required', () => {
   it('bubbles @required(action: LOG) scalars up to LinkedField', () => {
@@ -751,4 +751,44 @@ describe('RelayReader @required', () => {
     expect(isMissingData).toBe(true);
     expect(data).toEqual(null);
   });
+
+  it('bubbles to list item when used in plural fragment', () => {
+    const source = RelayRecordSource.create({
+      'client:root': {
+        __id: 'client:root',
+        __typename: '__Root',
+        'nodes(ids:["1","2"])': {__refs: ['1', '2']},
+      },
+      '1': {
+        __id: '1',
+        id: '1',
+        __typename: 'User',
+        username: 'Wendy',
+      },
+      '2': {
+        __id: '2',
+        id: '2',
+        __typename: 'User',
+        username: null,
+      },
+    });
+    const BarFragment = graphql`
+      fragment RelayReaderRequiredFieldsTest6Fragment on User
+      @relay(plural: true) {
+        username @required(action: LOG)
+      }
+    `;
+    const UserQuery = graphql`
+      query RelayReaderRequiredFieldsTest24Query {
+        nodes(ids: ["1", "2"]) {
+          ...RelayReaderRequiredFieldsTest6Fragment
+        }
+      }
+    `;
+    const owner = createOperationDescriptor(UserQuery);
+    const {nodes} = read(source, owner.fragment).data;
+    const pluralSelector = getPluralSelector(BarFragment, nodes);
+    const data = pluralSelector.selectors.map(s => read(source, s).data);
+    expect(data).toEqual([{username: 'Wendy'}, null]);
+  });
 });