chore(federation): remove for_each_element#5778
Merged
goto-bus-stop merged 1 commit intodevfrom Aug 6, 2024
Merged
Conversation
This set of functions is used in one place, and it has some odd behaviours. In JS, an "element" is free, because its data structures are built on element/selection set pairings. But we don't have the "element" type exactly in our data structures, our closest equivalent is the `Selection` type. Therefore the `for_each_element` function had to convert types including some inexpensive, but non-zero clones. `for_each_element` also translates named fragment spreads to inline fragments. AFAICT, this is a historical artifact ported from JS, as the only place where `for_each_element` was used does not support named fragment spreads to begin with. I replaced it, for now, with a manual visitor. I think it might make sense to have something like a `for_each_selection` API but it's honestly not that much harder to do it manually...
Contributor
|
@goto-bus-stop, please consider creating a changeset entry in |
|
CI performance tests
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today's 15-minute cleanup :)
This set of functions is used in one place, and it has some odd behaviours. In JS, an "element" is free, because its data structures are built on element/selection set pairings. But we don't have the "element" type exactly in our data structures, our closest equivalent is the
Selectiontype. Therefore thefor_each_elementfunction had to convert types including some inexpensive, but non-zero clones.for_each_elementalso translates named fragment spreads to inline fragments. AFAICT, this is a historical artifact ported from JS, as the only place wherefor_each_elementwas used does not support named fragment spreads to begin with.I replaced it, for now, with a manual visitor. I think it might make sense to have something like a
for_each_selectionAPI but it's honestly not that much harder to do it manually...