Feature request: Enable nova data façade to support partial implementations of the interface
ionmorozan opened this issue · 4 comments
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.
@alloy @ionmorozan Was this discussed in more detail? Did we resolve this request?
@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>
);
@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?
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