Skip to content
This repository was archived by the owner on Oct 11, 2022. It is now read-only.

Conversation

@brianlovin
Copy link
Contributor

Status

  • WIP
  • Ready for review
  • Needs testing

Deploy after merge (delete what needn't be deployed)

  • api
  • hyperion (frontend)

@spectrum-bot
Copy link

spectrum-bot bot commented Oct 29, 2018

Warnings
⚠️

These modified files do not have Flow enabled:

  • src/components/profile/coverPhoto.js

Generated by 🚫 dangerJS

@brianlovin brianlovin requested a review from mxstbr October 29, 2018 05:54
@brianlovin
Copy link
Contributor Author

Investigating why e2e are mostly failing...

@brianlovin
Copy link
Contributor Author

Somehow something in this PR is causing an infinite loop to be triggered on channel settings views. I'm at a loss for why, but will figure this out tomorrow when I have more energy .

@brianlovin
Copy link
Contributor Author

Ah, I figured it out. ccing @mxstbr for some brain juice on this. The problem is that when we sign image urls we are attaching an expiration timestamp. If that expiration timestamp changes during the same request, it blows up Apollo and causes infinite refetches.

Seems like the solution to this will be to have a single expiration time per request that gets set on all resolved images...perhaps this could be achieved by setting an imageExpirationTime field in context - would that make sense to you?

@brianlovin
Copy link
Contributor Author

Seems like the solution to this will be to have a single expiration time per request that gets set on all resolved images...perhaps this could be achieved by setting an imageExpirationTime field in context - would that make sense to you?

Ugh, I tried this but it still breaks because of Apollo's caching mechanism detecting changes between api requests and infinitely reloading.

Next thing to try would be futzing with Apollo's caching options to ignore profilePhoto and coverPhoto field changes...not sure about this.

@brianlovin
Copy link
Contributor Author

Uhhhh...it's entirely possible that this breaks SSR entirely, as well. Shit.

mxstbr
mxstbr previously approved these changes Oct 29, 2018
mxstbr
mxstbr previously approved these changes Oct 29, 2018
Copy link
Contributor

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YAS

@brianlovin
Copy link
Contributor Author

Finally lol

@brianlovin brianlovin merged commit 5a9f8b1 into alpha Oct 29, 2018
@brianlovin brianlovin deleted the image-signing branch October 29, 2018 17:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants