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

Missing release for removing redux-offline #1425

Open
lauraseidler opened this issue May 18, 2021 · 4 comments
Open

Missing release for removing redux-offline #1425

lauraseidler opened this issue May 18, 2021 · 4 comments

Comments

@lauraseidler
Copy link

lauraseidler commented May 18, 2021

Summary

It looks like redux-offline was already removed from the web app, so the actions in this package relying on it don't work anymore.

Environment Information

  • Webapp or React Native app: Web
  • Mattermost Server Version: mattermost-enterprise-edition:5.35.0
  • mattermost-redux Version: 5.33.1

Steps to reproduce

Create a plugin that uses the createPost action or any other action that depended on redux-offline.

Expected behavior

No API request to create the post is sent, and the post stays pending indefinitely on the creating client's side.

Observed behavior

The API request to create the post is sent, the post is persistet and visible for all members of the channel.

Possible fixes

If I'm not mistaken, this problem is caused by removing redux-offline in these PRs:

and changing mattermost-webapp to use its own version in this PR: mattermost/mattermost-webapp#7723

While I get the point of doing this, it looks like there was never a release of mattermost-redux that also removes redux-offline, as it's still bundled in the latest 5.33.1 release. This breaks plugins that e.g. use createPost from mattermost-redux, as the code still includes this part:

// ...
dispatch({
    type: action_types_1.PostTypes.RECEIVED_NEW_POST,
    data: tslib_1.__assign(tslib_1.__assign({}, newPost), { id: pendingPostId }),
    meta: {
        offline: {
            effect: function () {
                return client_1.Client4.createPost(tslib_1.__assign(tslib_1.__assign({}, newPost), { create_at: 0 }));
            },
// ...

As redux-offline was removed from the web app and doesn't look to be bundled with mattermost-redux, the meta.offline part is simply never executed.

This could be somewhat "fixed" by releasing this change for mattermost-redux, however that would still mean plugins relying on these actions that are not updated would remain broken.

Addendum: I can comfirm that building mattermost-redux from the current master of this repo and using those files instead of npm installing mattermost-redux in a plugin successfully works around this issue - not the best idea tho, IMO.

@amyblais
Copy link
Member

cc @hmhealey / @lieut-data

@hmhealey
Copy link
Member

Ah, we were worried that might happen. We unfortunately never set up mattermost-redux in a way that makes it easy to ensure backwards compatibility for things like this. We have some future plans for a safer replacement for mattermost-redux that will be provided to a plugin by the web app directly, but that won't be coming for a while.

While using the latest master doesn't feel great, that's probably your best bet for keeping createPost usable. Our web app referenced mattermost-redux in its package.json by a git commit instead of a published version until very recently, and many of our internally-maintained plugins do the same. In particular, our paid Incident Collaboration plugin points to the latest master, so you can trust we're not going to be making any more breaking changes to that any time soon.

@lauraseidler
Copy link
Author

@hmhealey thanks for bringing some clarity!

I understand your reasoning, tho I would've personally wished for a note somewhere on this - when I saw this repo/package, I didn't even think that it might not be intended to be used this way, and I assumed it would still be stable while being migrated into the web app.

@hmhealey
Copy link
Member

Funny you should mention that since it was the original vision of the package, but unfortunately, it got out of hand quickly with a huge amount of stuff being added that we couldn't ensure compatibility with.

Unfortunately, the redux-offline removal was an important performance improvement, and while we were aware it could break someone, we had kind of hoped the area it affected was small enough that it wouldn't actually affect anyone. We were definitely wrong there.

In the longer term, we'd like to get a better API for plugins to access Mattermost data that's easier to keep compatibility with (aka it'll be smaller, cleaner, and provided by the web app to plugins directly), but that's unfortunately not a priority at the moment. That said, I'll try to help out where I can to get things working.

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

No branches or pull requests

3 participants