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

Feature request: Enable nova data façade to support partial implementations of the interface #31

Open
ionmorozan opened this issue Feb 8, 2022 · 4 comments

Comments

@ionmorozan
Copy link

Hi,

Scenario
Org Explorer is an Apollo graphql people experience (aka.ms/orgexplorer) where my team integrated People Highlights (ref on scenario here). Our integration package is implementing the nova data interface.

Background info
Today highlights are integrated in the host app (OrgX) root query. OrgX is a NON-nova host app. Highlights type is embedded inside OrgX root query making the implementation of useLazyLoadQuery difficult. so far we opted for implementing the following methods, while querying happens inside the host app and then it's passed down as props:

  • useMutation - this works well because highlights are not embedded in the orgx root mutation (it's decoupled)
  • useFragment - we are relying on the default implementation where data passed via props is mirrored.

Challenge
Since data is passed down via props, we can't ensure proper usage of useFragment unless we extract from the Fragment$key the data and forcefully cast it to a ref.

const highlightRef = props.personaHighlightFragment[" $data"] as unknown as {
    readonly " $fragmentRefs": FragmentRefs<"HighlightsFragment">;
  };

where personaHighlightFragment is a Nova fragment constructed this way (on the host app)

 // Convert from Apollo fragment to Nova(Relay) fragment
  const personaHighlightFragment: NovaHighlightsFragment$key = {
    " $fragmentRefs": { HighlightsFragment: true },
    " $data": personaHighlight as NovaHighlightsFragment,
  };

Ask: Enable nova to handle such object transformations when data is not fetched via useLazyLoadQuery but rather passed via props.

@kerrynf
Copy link
Contributor

kerrynf commented May 2, 2022

@alloy @ionmorozan Was this discussed in more detail? Did we resolve this request?

@ionmorozan
Copy link
Author

@kerrynf , short answer is no. We have not taken this to the next level of details. Today we opted for casting the hostApp specific fragment to a Nova defined fragment. See below what I actually mean by it.

   // Compose the highlight fragment ref.
  // This is a workaround to mimic the response we would get from useLazyLoadQuery.
  // https://github.com/microsoft/nova-facade/issues/31
  const highlightRef = props.personaHighlightFragment as unknown as {
    readonly " $fragmentRefs": FragmentRefs<"HighlightsFragment">;
  };

  return (
    <HighlightsPopupHintProvider {...props}>
      <Highlights highlightRef={highlightRef} size={props.size} />
    </HighlightsPopupHintProvider>
  );

@sjwilczynski
Copy link
Contributor

@kerrynf, @alloy +1 on this issue. This is particularly a problem when you want to create some simple stories in Storybook for a component that gets data through useFragment. Let's take a component as simple as:

export type PersonProps = {
  person: Person_personFragment$key;
};

export const personFragment = graphql`
  fragment Person_personFragment on Person {
    defaultName {
      displayName
    }
  }
`;

export const Person = (props: PersonProps) => {
  const person = useFragment(personFragment, props.person);
  const name = person.defaultName?.displayName ?? "";
  return (
    <div>
      <Avatar
        className={styles.avatar}
        name={name}
        active={"active"}
        activeAppearance={"shadow"}
        color={"neutral"}
        shape={"circular"}
        size={40}
      />
        <Text>{name}</Text>
    </div>
  );
};

Normally I would like to be able to pass data through args and it is possible as useFragment by default would pass whatever is put in as fragmentRef. But to get typescript to be happy we need to add cast as unknown as Person_personFragment$key, which makes stories not typesafe because with explicit cast like this we won't get any type check failures on changing fragments (removing/adding fields)

export default {
  component: Person,
  decorators: [/* some decorators*/],
};

type Story = ComponentStoryObj<typeof Person>;

export const Primary: Story = {
  args: {
    person: {
      defaultName: {
        displayName: "John Doe",
      },
    } as unknown as Person_personFragment$key,
  },
};

Maybe we could have some utility function that would allow us to create data for fragment declaratively but still be able to satisfy type system?

@sjwilczynski
Copy link
Contributor

Storybook part is solved with the decorator available in @nova/react-test-utils. As for partial implementation of the interface we haven't discussed this further but I'd say we shouldn't allow to pass fragment raw prop, it should come from some graphql query executed somewhere

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

No branches or pull requests

3 participants