fix: Allow readFragment to be called on already unmasked fragments

[kitten]

Supersedes #121
cc @benjie

Summary

This allows readFragment to be called on already unmasked fragments, which will most likely happen if we try to call readFragment on data that's already typed by (i.e. has already been unwrapped by) another readFragment call.

See #121 for more details

Set of changes

• Add overload to readFragment that accepts already unwrapped data

[changeset-bot]

🦋 Changeset detected

Latest commit: 41ebcce

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[benjie]

I don't think this actually solves the issue. Consider:

import { FC } from "react";

const MyComponent: FC<{
  user1: FragmentOf<typeof UserFrag>;
  user2: FragmentOf<typeof UserFrag>;
}> = ({ user1: data1, user2: data2 }) => {
  const user1 = readFragment(UserFrag, data1);
  const user2 = readFragment(UserFrag, data2);

  const winner = user1.score > user2.score ? user1 : user2;

  return <OtherComponent user={winner} />;
};

const OtherComponent: FC<{ user: FragmentOf<typeof UserFrag> }> = ({
  user: data,
}) => {
  const user = readFragment(UserFrag, data);
  return <strong>{user.name}</strong>;
};

The issue isn't the readFragment a second time (though that may be an issue), it's the passing of winner to <OtherComponent user={winner} /> instead of going back to the raw data <OtherComponent user={winner === user1 ? data1 : data2} />.

(Poor example since OtherComponent should use it's own fragments, but consider passing this to utility functions and what not which would want to declare data in the same way.)

Intuitively it feels like you should be able to pass a readFragment'd value into a FragmentOf<typeof Frag> parameter.

[kitten]

I'm not 100% sure that's a good idea though, because it basically blurs the line between the type containing the “virtual” properties that a masked fragment produces and the non-virtual ones, i.e. the selected fields.

The issue I'm basically having is that when readFragment is called and we're passing on that data instead of what readFragment received (the masked data), that's almost always an accidental mistake, since in theory the variable holding the masked data will also be available in the same scope.
So, while it may be an intuitive pattern to say both are interchangeable it causes some minor issues

I'm basically thinking of, for example, keyof specifically (or mapped types in general),

Here, I'm adding keyof typeof result where result is the output from readFragment. The selection here has been unmasked, but if we were to re-add the fragment mask to it, we'd get excessive keys.

That's what scares me a little. Our “fragment ref” property type, is a unique symbol on purpose. We're trying to minimise the chance of it being confused for an actual, real property that the data can have. If we were to always have FragmentOf be a part of the result type here we may risk making the “pure” data type completely inaccessible to users.

[benjie]

Is there a type helper for the unwrapped version of a fragment?

[kitten]

@benjie: ResultOf should work universally, both for fragment documents and for operation documents

[kitten]

I'll merge this for now, since I think this is a good change either way. But I'll keep that comment in mind.

I think what we could ponder is whether FragmentOf<> should actually be an alias of ResultOf<> | T. The problem I had with that was that it might be unreasonably easy to “shake off” the T i.e. fragment mask type then.

But there must be a middleground here somewhere 🤔