fix: don't include view stroke by default for non-cartesian plot #7665
Conversation
kanitw
changed the title
kanitw
force-pushed
the
kw/7621-default-view-stroke
branch
2 times, most recently
from
ae3c6e0
to
95844f4
Compare
kanitw
force-pushed
the
kw/7621-default-view-stroke
branch
from
b061053
to
df60ede
Compare
| uniqueStyles.add(style); | ||
| } | ||
| } | ||
| const styles = Array.from(uniqueStyles); |
There was a problem hiding this comment.
I think you can check the length of the set and return the first item of the set instead. No need to make it an array.
There was a problem hiding this comment.
To get the first item you need to get the iterator and call next, which seems equally expensive as this but result in a more complicated code. So I think this is actually better.
There was a problem hiding this comment.
I disagree. The array call makes a copy while creating an iterator is highly optimized.
There was a problem hiding this comment.
I guess since we may return an array, it doesn't make a big difference.
There was a problem hiding this comment.
Most of the time N=1 or a few. Maybe iterator is marginally better, but produces way longer code that doesn't justify the benefit.
…a#7665) Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
fix #7621