Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore non postmate message during handshake #35

Merged
merged 2 commits into from
Dec 5, 2017

Conversation

dboskovic
Copy link
Contributor

If a message is received that does not have a postmate payload, it will break the handshake. This is likely the cause of #29 and #31. It's difficult to add a test for this since it only fails if the message happens exactly in between the handshake.

Below is an example of the handshake failing due to a webpackOk event.

image

@dboskovic
Copy link
Contributor Author

@yowainwright @jakiestfu Not sure if you want the build file added in the PR. The minor dep changes are due to some bit-rot with rollup (lemmabit/rollup-stream#19) and a missing babel dep.

I'm not sure why build changed the files size by a kb for my condition. Let me know if you'd like anything revised here.

@jakiestfu
Copy link
Contributor

jakiestfu commented Oct 30, 2017

Would you mind doing a little investigating with regards to the additional kb @dboskovic?

@eyalcohen4
Copy link

Hey, we really need this pull request to be merged. How can we help?
cc @dboskovic

@dboskovic
Copy link
Contributor Author

@eyalcohen4 the only weird thing was the full kb increase in size of the compiled file (or at least the size that was published to the readme). Not sure what to do there. There's nothing other than gulp build that I can see for doing this. 🤔

@dboskovic
Copy link
Contributor Author

dboskovic commented Dec 3, 2017

@jakiestfu after a little research I think it's related to the "external-helpers" plugin for babel. Since it wasn't previously in package.json I installed the latest version which may have more helpers than the version you guys last compiled with. I tried rolling it back to the previous major version but it's incompatible with Babel 6. ¯\(ツ)

@yowainwright
Copy link
Contributor

@dboskovic I think a pr that was merged recently should help rather than hurt.

Could you rebase please? 🙏

@dboskovic
Copy link
Contributor Author

done!

@yowainwright yowainwright merged commit 98a4db6 into dollarshaveclub:master Dec 5, 2017
@yowainwright
Copy link
Contributor

@dboskovic please let me know if all is good! Thanks for your patience and time. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants