feat(Nav): performance improvements#55
Conversation
kmcfaul
changed the title
| ] | ||
| } | ||
|
|
||
| navData[section] = navEntries; |
There was a problem hiding this comment.
I'm not 100% sure, but I think because the sections were previously sorted it maintains that order as it gets added to the data (because it is ordered by insertion order). LMK if I'm incorrect about this.
There was a problem hiding this comment.
I think insertion ordered isn't guaranteed to be preserved in all browsers, but I'm also not 100% sure.
I wonder if we should form and pass the nav data as an array rather than as an object since in the nav we just map through the entries anyways.
|
🎉 This PR is included in version 1.5.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
| ) | ||
|
|
||
| const sortedSections = [...orderedSections, ...unorderedSections.sort()] | ||
| sortedSections.map((section) => { |
There was a problem hiding this comment.
Feels kind of weird that we're doing a map here if we aren't used the mapped result? Or am I misunderstanding something.
| ] | ||
| } | ||
|
|
||
| navData[section] = navEntries; |
There was a problem hiding this comment.
I think insertion ordered isn't guaranteed to be preserved in all browsers, but I'm also not 100% sure.
I wonder if we should form and pass the nav data as an array rather than as an object since in the nav we just map through the entries anyways.
|
Oh I see this was merged between before I finished the review 😬 |
3 participants
Open question/thoughts:
I think we can probably clean up the data structure that Navigation gets a bit more, the TextContentEntry probably doesn't need the
data.sectionnow because that info is in the Record, and it doesn't have to match the entry data structure (so we can probably move away fromid, data: {id}style).