performance: Move API calls out of render and into subway_status - #3408
performance: Move API calls out of render and into subway_status#3408jlucytan wants to merge 13 commits into
Conversation
joshlarson
left a comment
joshlarson
left a comment
There was a problem hiding this comment.
I like what I see so far, and having these calls not run per-component-load is going to be a huge win!
However, I think something's gone a bit wrong with combining subheadings. Compare dev-green on the left to this branch on the right.
It looks like status on the alerts page is combining subheadings from different rows even when the rows themselves aren't being combined, but on that page, each row should only show the affected stops or endpoints from the alert associated with that row.
|
|
||
| defp decorations(%{status: status, alerts: alerts, route_ids: route_ids}) | ||
| defp decorations(%{status: status, subheading_data: {:endpoint_stops, endpoints}}) | ||
| when status in [:service_change, :shuttle, :single_tracking, :suspension] do |
There was a problem hiding this comment.
Suggestion (non-blocking): I think we can (and should) remove the when status in check here - now that you've moved the "which statuses get endpoints?" check into the backend, the frontend can just be like "endpoints? okay I'll draw them like this"
| prefix={@row.status_entry.prefix} | ||
| plural={@row.status_entry.plural} | ||
| future={@row.status_entry.future} | ||
| subheading_data={@subheading_data} |
There was a problem hiding this comment.
Suggestion (non-blocking): If you add subheading_data: nil to the "normal status" clause of rows_for_status_entry/3, then every status_entry map at this point in the code will have a subheading_data field defined, and then you can replace this with 👇, similar to all the other fields here, and get rid of the assigns-fiddling you're doing above.
<.status_row_heading
# ...
subheading_data={@row.status_entry.subheading_data}
/>2 participants
Scope
Asana Ticket: 📈 🛠️ [Subway Status] Move API calls / stop-loading out of component-rendering
Implementation
Moves
affected_stopsandendpoint_stopsout ofstatus_row_headingand intosubway_statusso there are fewer API calls at render time on the home page, and the data is cached.The alerts page and schedule finder still make calls to populate the subheading for the Planned Work section, however.
Although commuter_rail_upcoming_changes also utilizes the
status_row_headingcomponent, I don't believe it currently shows any subheadings, so no data is added there.Screenshots
Should have no visual changes.
Old flamegraph:
New flamegraph:
How to test
Homepage, alerts page, schedule finder alert banners, commuter rail upcoming changes appearances should not change between this branch and prod.