Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

Reverting serialize-error to version 2.1.0 #822

Merged
merged 1 commit into from
Apr 29, 2019

Conversation

jespino
Copy link
Member

@jespino jespino commented Apr 25, 2019

Summary

Reverting serialize-error to version 2.1.0

serialize-error is though for node, and the version 3.0.0 is using ES6 directly, without transpiling anything (added the Node 6 requirement) and it breaks IE11, so... we must stick to 2.1.0 or remove entirely the dependency (is used in one single place, the logError action.

Ticket Link

MM-15285

@jespino
Copy link
Member Author

jespino commented Apr 26, 2019

Other option would be embed the serialize-error code entirely in our code (is not too much code, only 2 functions) and we can remove the dependency entirely, avoiding future upgrades to screw us.

@jespino
Copy link
Member Author

jespino commented Apr 26, 2019

Now I'm starting to think this is more related to babel upgrade, or something like that. I'm still investigating

@jespino jespino added the Do Not Merge Should not be merged until this label is removed label Apr 26, 2019
@jespino
Copy link
Member Author

jespino commented Apr 26, 2019

Finally I can test it, and is a combination of 4 things:

    1. the development compilation generate a security error (which is not important, because the production build doesn't execute that code).
    1. The serialize-error dependency here.
    1. the exif2css dependency in mattermost-webapp.
    1. a error accessing plugin.initialize when a plugin has fail to initialize.

So I going to make another PR in mattermost-webapp to fix everything else.

@jespino jespino requested a review from crspeller April 26, 2019 14:02
@jespino jespino added 2: Dev Review Requires review by a core commiter and removed Do Not Merge Should not be merged until this label is removed labels Apr 26, 2019
@jespino jespino requested a review from hmhealey April 26, 2019 14:02
@jespino jespino requested review from jwilander and removed request for crspeller April 26, 2019 14:11
@hmhealey hmhealey merged commit bba3477 into mattermost:master Apr 29, 2019
@hmhealey hmhealey added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core commiter labels Apr 29, 2019
@hmhealey hmhealey added this to the v5.12.0 milestone Apr 29, 2019
@lindy65 lindy65 added Tests/Not Needed Does not require tests and removed 4: Reviews Complete All reviewers have approved the pull request labels May 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Tests/Not Needed Does not require tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants