`updates_for` Makes Redundant Calls

[andrewerlanger]

I think the order in which an updates_for_element checks whether it shouldUpdate may be a little off.

Let's say I have the following html:

<%= updates_for @item, only: :foo do %>
  ...
<% end %>

And then I update a field not specified in the only option:

@item.update(bar: true)

If we look at the update(data) method of updates_for_element.js, the logic will play out in the following order:

1. We create a new Block out of the element (L49).
2. We fetch the html for the element (L65). This part is the problem, I think. Ideally, we shouldn't be requesting the html until we know the element should actually be updated, but we haven't run that check yet.
3. We process the block (L85). It's only now that we realize the element shouldn't be updated, and so we don't proceed with the actual updating. But this means the fetch in step 2. above was redundant: we asked the server to render the full html for the page but didn't do anything with it.

I think the better approach here would be to check whether the update should be performed before we go and pester the server, so something along the lines of:

let blocks = Array.from(
  document.querySelectorAll(this.query),
  element => new Block(element)
)

// filter out any blocks that don't require updates
blocks = blocks.filter(block => block.shouldUpdate(data))

// return here if there are no blocks that require updates
if (blocks.length === 0) return

// carry on with the fetch logic only for blocks that require updates
...

Not sure if I'm missing something here, but I played around with the above locally and it seemed to work well.

@julianrubisch @leastbad penny for your thoughts :)

[leastbad]

Just wanted to say: we are looking at this! Sorry for the radio silence.