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

Conversation

@brianlovin
Copy link
Contributor

@brianlovin brianlovin commented May 29, 2018

Status

  • WIP
  • Ready for review
  • Needs testing

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

  • api
  • hyperion (frontend)
  • hermes

Working through some sentry bugs.

f147761 => https://sentry.io/space-program/spectrum/issues/558113660/?query=is:unresolved

5e4479a => https://sentry.io/space-program/spectrum/issues/564742242/?query=is:unresolved

a3be231 => https://sentry.io/space-program/spectrum/issues/545394207/?query=is:unresolved and https://sentry.io/space-program/spectrum/issues/562870198/
cc @mxstbr ^^ => I don't want to go too far down the rabbit hole of supporting super old versions of IE, but we are seeing a fairly regular stream of bugs affecting edge and even IE11. We can simply extend the default polyfills provided by polyfill.io as seen in this commit, although I'm not sure what other things we'll have to keep adding to this over time. I spent a while this afternoon trying to find a more cohesive way to polyfill this kind of stuff, but obviously it just adds complexity if we try to keep bundle size small for users on modern browsers. The two added in this commit add a few hundred bytes for bugs affecting maybe a few dozen users. Wdyt?
Documentation: https://polyfill.io/v2/docs/examples#aliases
Actually, this is affecting hundreds of users: https://sentry.io/space-program/spectrum/issues/562170782/?query=is:unresolved

e4a3538 => https://sentry.io/space-program/spectrum/issues/565877960/?query=is:unresolved

0ad80a1 => https://sentry.io/space-program/spectrum/issues/530053952/events/21267481078/ - this one is pretty important as we have some invalid emails getting into our db which have been causing trouble in hermes (to the tune of 257 events in the last 30 days)

b50082d => https://sentry.io/space-program/spectrum/issues/513764806/?query=is:unresolved - @mxstbr let's chat about this one specifically. Any thoughts on how we could properly flowtype all of our loaders? Flow should have caught that we don't have an isDeleted field on a DBThread 😅

f706731 => this is a temporary fix to prevent https://sentry.io/space-program/spectrum/issues/566970820/?query=is:unresolved from crashing the frontend. Issue opened at https://github.com/withspectrum/spectrum/issues/3190 to discuss the proper fix

00d5808 => https://sentry.io/space-program/spectrum/issues/498612908/?query=is:unresolved - additionally I've improved some of the default behaviors when a person presses esc or clicks outside of the search results :)

@brianlovin brianlovin changed the title Ensure event exists before attempting to read clipboard data Bug bash day May 29, 2018
@spectrum-bot
Copy link

spectrum-bot bot commented May 30, 2018

Warnings
⚠️

These modified files do not have Flow enabled:

  • src/components/threadComposer/components/composer.js

Generated by 🚫 dangerJS

@brianlovin
Copy link
Contributor Author

Kk this is ready for review + discussion @mxstbr - each commit is pretty self contained. Let's discuss about:

  • polyfilling IE to handle Array.find and Symbol.iterator
  • how to best flowtype our loaders

@mxstbr
Copy link
Contributor

mxstbr commented May 30, 2018

Any thoughts on how we could properly flowtype all of our loaders?

Yes, I looked into this but it's hard and not worth the effort atm imo.

polyfilling IE to handle Array.find and Symbol.iterator

I thought the point of polyfill.io was that it automatically detects which browser a user's using and only sends them the most minimal polyfill? Oh well if this fixes things it's fine with me!

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.

LGTM, thank you!

@mxstbr mxstbr merged commit 03e7ae7 into alpha May 30, 2018
@mxstbr mxstbr deleted the bug-bash-day branch May 30, 2018 07:59
@brianlovin
Copy link
Contributor Author

I thought the point of polyfill.io was that it automatically detects which browser a user's using and only sends them the most minimal polyfill?

I thought so, too :/

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