Skip to content

Conversation

@kitten
Copy link
Member

@kitten kitten commented Mar 11, 2024

Note

It's debatable whether this is a feature or a fix, but since it's a relatively safe addition that complements #98 (for #77) and that we can safely roll back if needed, I'd like to try shipping this as a patch to evaluate this quickly.

Summary

This allows the document in readFragment to be passed as a generic, removing the need to keep fragments around as a runtime value.

// previous:
const unmaskedData = readFragment(document, maskedData);
// new:
const unmaskedData = readFragment<typeof document>(maskedData);

Replacing readFragment's complex type argument and data generic with an overload is long and wordy, but potentially makes it simpler for TypeScript to evaluate and prevents us having to maintain mirrorTypeRec.

In case this is needed, we can add its definition back to the overload, but this wouldn't be compatible with being able to pass just one generic for the document.

Set of changes

  • Replace maskFragments and readFragment definitions with overloads
  • Allow readFragment to only accept one argument

@kitten kitten requested a review from JoviDeCroock March 11, 2024 11:20
@changeset-bot
Copy link

changeset-bot bot commented Mar 11, 2024

🦋 Changeset detected

Latest commit: e0e4f7a

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

@kitten kitten changed the title fix: Feat/allow generic fragments fix: Allow readFragment() to accept fragment document as a generic Mar 11, 2024
@kitten kitten merged commit dcc6ebc into main Mar 11, 2024
@kitten kitten deleted the feat/allow-generic-fragments branch March 11, 2024 13:20
@github-actions github-actions bot mentioned this pull request Mar 11, 2024
@kitten kitten added this to the Are We Fast Yet? milestone Mar 22, 2024
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

Successfully merging this pull request may close these issues.

3 participants