removed repeatedly refetching remote pins #2105
Conversation
|
@JustinTime42 thanks so much for submitting this fix. That playback bug was something I kept wanting to come back to but haven't had the chance. I will confirm the fix today and try to get this merged. |
|
I updated the code so that we can still poll for fetch updates, but not re-render if there are no updates. This is not as performant as it could be, but it resolves the issue. Can you confirm @JustinTime42 ? Also requested a review from @whizzzkid since this is basically only my code now. |
src/bundles/pinning.js
Outdated
| const pins = [ | ||
| ...store.selectPendingPins(), | ||
| ...store.selectFailedPins() | ||
| ].map(serviceCid => ({ cid: serviceCid.split(':')[1] })) | ||
| store.doFetchRemotePins(pins, true) | ||
| } |
There was a problem hiding this comment.
code seems straightforward to me. It makes sense that if this is being called every 30 seconds (PIN_CHECK_INTERVAL), that any components dependent upon doFetchRemotePins would refresh when this is called. However, if there are no updates, we shouldn't be updating the store and there should be no update.. so this continually updating means the correct fix is probably in doFetchRemotePins.
| */ | ||
| const getRerenderAwareArray = (oldArray, newArray) => { | ||
| if (oldArray.length !== newArray.length) return newArray | ||
| const diff = oldArray.filter((i) => !newArray.includes(i)) |
There was a problem hiding this comment.
Sooo... what happens if the newArray has elements that oldArray doesn't? i.e. if the oldArray is a subset of newArray then diff would be empty?
There was a problem hiding this comment.
if the length different, we always return the new array.
if the length is the same, we compare content.
so in your example, oldArray is a subset, meaning length is different, meaning we return the new array
|
🎉 This PR is included in version 3.0.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This fixes #2080. I can't find any reason in the current code base for why
doFetchRemotePinsneeds to repeat continuously, and doing so causes a periodic re-render that interrupts media playback in the file preview.