Skip to content

Breaking changes after updating extension to v3 #1002

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

Closed
j-dowell opened this issue Jan 10, 2022 · 34 comments · Fixed by #1003
Closed

Breaking changes after updating extension to v3 #1002

j-dowell opened this issue Jan 10, 2022 · 34 comments · Fixed by #1003
Labels

Comments

@j-dowell
Copy link

After updating the Redux Dev Tools extension (Chrome) to v3 this weekend, our app is no longer working when the extension is enabled. Still works on Firefox as the add on hasn't been updated yet.

We use redux-most as middleware, like so:

import { createStore, compose, combineReducers } from 'redux'
import { combineEpics, createEpicMiddleware, createStateStreamEnhancer } from 'redux-most'

const composeEnhancers = window.__REDUX_DEVTOOLS_EXTENSION_COMPOSE__ || compose

const epicMiddleware = createEpicMiddleware(
    combineEpics([
        appEpic,
        ... // other epics here
    ])
)

const store = createStore(
    combineReducers({
        appReducer,
        ... // other reducers here
    }),
    composeEnhancers(createStateStreamEnhancer(epicMiddleware))
)

export default store

And now when accessing state (using withState), instead of the regular state object being returned it looks something like this:

{
    actionsById: {...},  
    committedState: undefined, 
    computedStates: [{...}],
    currentStateIndex: 1,
    isLocked: false,
    isPaused: false,
    monitorState: {},
    nextActionId: 2,
    skippedActionIds: [],  
    stagedActionIds: [0, 1, ...],
}

Where the actual state object can be located at the last index of computedStates

Does anyone have any ideas as to what happened here and how it could be resolved? Thanks

@markerikson
Copy link
Contributor

Oh dear.

A couple quick questions to get a sense of what's going on:

  • What happens if you just console.log(store.getState())? Is it also this "lifted state" value from the DevTools?
  • What happens if you skip the composeWithEnhancers part?
  • Finally, what about using composeWithEnhancers and skipping the middleware?

@Methuselah96
Copy link
Member

I'm able to reproduce the issue with the example project for redux-most, looking into it.

@Methuselah96
Copy link
Member

@j-dowell I believe I have a solution. There was an observable property leaking through the Redux DevTools store that was exposing the lifted state.

However if I release the fix, I'm concerned that your app will break even worse. In my testing it seemed that redux-most could not handle Redux versions >=4.1.0 due to the changes in how the observable symbol was ponyfilled. In those cases, the app white-screens and the same thing would happen if I released the fix.

What version of Redux are you currently on? If you're on a version less than 4.1.0 can you try updating and see if breaks your app?

@markerikson
Copy link
Contributor

Huh. What's the observable ponyfill issue, specifically?

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

The issue is:
Redux 4.0.5 depends on symbol-observable as does most. symbol-observable polyfills Symbol.observable and so it doesn't matter which one calls symbol-observable first, because the first one to access will do the polyfill and therefore both libraries will share the same symbol.

Redux 4.1.0 does not depend on symbol-observable and is instead just doing a ponyfill for the observable symbol. Therefore if redux is imported first, it sees that Symbol.observable is not set and decides to use '@@observable' as the ponyfill. However when symbol-observable is imported later, it polyfills Symbol.observable and that is what is used by most. Because of that the ponyfill and polyfill observable symbol properties don't match up and most crashes because it was expecting the Redux store to have the Symbol.observable polyfill property.

So it seems like Redux should polyfill Symbol.observable if it wants to remain compatibility with libraries that use symbol-observable (perhaps add back the dependency on symbol-observable for simplicity?).

@markerikson
Copy link
Contributor

Grrr. We specifically removed that dependency in 4.1.0 specifically because it was a lousy 1-line dependency and I didn't want to have to rely on it.

Could we just advise users to import the polyfill first if it's a thing they need to depend on?

@Methuselah96
Copy link
Member

Was the dependency causing any specific problems? It seems like adding a few bytes to the bundle size is easily worth it in this case in order to avoid the special instructions of having to import the polyfill first, any complications that arise from that, as well as any developer time spent trying to figure out why it's broken. It's a pretty bad gotcha and easily solved on the Redux side of things. I like getting rid of dependencies as much as the next person, but this one seems like a net positive.

@markerikson
Copy link
Contributor

No specific problems, but it seemed unnecessary to have a dependency for that one-line polyfill, and I've been advocating for years that the JS ecosystem should cut down on tiny dependencies to help consolidate and improve installation. So, I was trying to follow my own advice there.

I don't want to re-add that dependency, but I'm open to discussion on it.

@sorinpav
Copy link

sorinpav commented Jan 11, 2022

Hi! Is there a way to downgrade to a previous version? We're having issues as well, and I'm not sure what to do to make it work again. It seems drastic to force us to update our code, when an extension is updated without our consent. I magically found the latest version of the extension on my system, without purposefully upgrading.

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

@sorinpav Unfortunately Chrome Extensions automatically updates extensions without consent and doesn't have a way to downgrade which is a severe limitation of Chrome Extensions. For now the best way to work around it is to download the extension.zip artifact from the most recent release and load it unpacked. Hopefully we can get this resolved as soon as possible.

Another possible workaround would be to try to import $$observable from 'symbol-observable'; as the first import in your app. What version of Redux are you using?

@sorinpav
Copy link

Thank you so much! That sorted our redux devtools extension. It's really frustrating to see breaking changes like that, hopefully it gets sorted out soon.

The redux version I'm using is 4.1.2.

@j-dowell
Copy link
Author

j-dowell commented Jan 11, 2022

@j-dowell I believe I have a solution. There was an observable property leaking through the Redux DevTools store that was exposing the lifted state.

However if I release the fix, I'm concerned that your app will break even worse. In my testing it seemed that redux-most could not handle Redux versions >=4.1.0 due to the changes in how the observable symbol was ponyfilled. In those cases, the app white-screens and the same thing would happen if I released the fix.

What version of Redux are you currently on? If you're on a version less than 4.1.0 can you try updating and see if breaks your app?

@Methuselah96 Thanks for looking into it. But our app is already using 4.1.1 of redux with no issues.

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

Thanks for the info. My guess is that the reason it breaks with the extension but not with plain Redux is because the order of import is the Redux DevTools extension, most/RxJS, and then Redux. I have a fix in mind that I'll try to get out today.

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

@sorinpav Are you also using redux-most and/or most.js?

@sorinpav
Copy link

@Methuselah96 no, I'm not using any of that

@Methuselah96
Copy link
Member

@sorinpav Got it, what problems are you having with the new extension?

@sorinpav
Copy link

@Methuselah96 the new extension just seems to not show any state whatsoever. I get state:undefined if I remember correctly. Maybe I haven't set things up properly, but the existence of this issue tells me I'm not the only one with this problem.

@a-c-sreedhar-reddy
Copy link

Hi @Methuselah96
The internal object from redux is being returned in my case.

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

@sorinpav Got it, sorry to hear you're having problems. This root issue of the OP is very distinctly caused by a weird interaction with symbol-observable and the extension that should only affect people who are using symbol-observable or a library that uses it like most.js. It's likely your root issue is different. If you could open a new issue with a description that shows the code you're using to set up your Redux store that would be extremely helpful. Thanks!

@a-c-sreedhar-reddy Sorry to hear that. Are you using any libraries that depend on symbol-observable and can you show me the code you're using to set up your Redux store?

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

@a-c-sreedhar-reddy Also, are you getting the internal object when calling .getState() or seeing it some other way?

@sorinpav
Copy link

@Methuselah96 i will open something up tomorrow morning if that's ok! Once again, thank you for all your help tonight! I'll be back tomorrow!

@Methuselah96
Copy link
Member

@j-dowell Can you try out Redux DevTools - Next when you get a chance? It is a beta extension that includes #1003. I would expect it to crash your app if I understand the situation correctly and I just want to make sure it does what I expect so I can decide how to proceed.

@j-dowell
Copy link
Author

j-dowell commented Jan 11, 2022

@Methuselah96 Sure, just installed and it crashed with:

from.js:31 Uncaught TypeError: from(x) must be observable, iterable, or array-like: [object Object]
    at from (from.js:31:1)
    at createStateStreamEnhancer.js:19:47
    at <anonymous>:10606:19
    at createStore (redux.js:154:1)
    at Module../src/redux/store.js (store.js:31:26)
    at __webpack_require__ (bootstrap:79:1)
    at Module../src/index.jsx (index.jsx:1:1)
    at __webpack_require__ (bootstrap:79:1)
    at checkDeferredModules (bootstrap:45:1)
    at bootstrap:152:1

@a-c-sreedhar-reddy
Copy link

a-c-sreedhar-reddy commented Jan 11, 2022

Are you using any libraries that depend on symbol-observable

@Methuselah96 Yes we added import 'symbol-observable'; in our polyfill.ts of our angular app.

can you show me the code you're using to set up your Redux store

This is how we instantiate redux store.

const composeEnhancers = composeWithDevTools(options);
export const globalStore = createStore(
  rootReducer,
  composeEnhancers(
    applyMiddleware(
      ...getDefaultMiddleware({
        thunk: true,
        immutableCheck: false,
        serializableCheck: false,
      }),
    ),
    // other store enhancers if any
  ),
);

are you getting the internal object when calling .getState() or seeing it some other way?

We wrap store in a from from rxjs ie from(globalStore) and subscribing to this gives the internal object.

@markerikson
Copy link
Contributor

@a-c-sreedhar-reddy : while I don't think it's directly related, that is a realllly weird way to set up a Redux store. It looks like you're mixing pieces of the plain createStore function with RTK's getDefaultMiddleware. Please don't do that :) Just use configureStore - it will do all that setup work for you.

(Also I'd be interested in hearing, in a separate discussion thread, why you opted you turn off the dev check middleware.)

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 11, 2022

@a-c-sreedhar-reddy Thanks, that's really helpful. A couple follow-up questions:

  1. Can you try out Redux DevTools - Next as well when you get a chance? I'm 90% your problem is the same, just want to make sure. I expect it to crash your app.
  2. Is it necessary for you to use symbol-observable and if so, what is it used for? RxJS has its own ponyfill for Symbol.observable if I understand correctly and so it shouldn't be necessary to use symbol-observable if you're just using it for RxJS.

@a-c-sreedhar-reddy
Copy link

a-c-sreedhar-reddy commented Jan 11, 2022

  1. Can you try out Redux DevTools - Next as well when you get a chance?

@Methuselah96 Yes it crashed
image

react_devtools_backend.js:4045 Unhandled Promise rejection: You provided an invalid object where a stream was expected. You can provide an Observable, Promise, Array, or Iterable. ; Zone: <root> ; Task: Promise.then ; Value: TypeError: You provided an invalid object where a stream was expected. You can provide an Observable, Promise, Array, or Iterable.
    at subscribeTo (subscribeTo.js:27)
    at from (from.js:9)

@Methuselah96
Copy link
Member

Methuselah96 commented Jan 12, 2022

@j-dowell @a-c-sreedhar-reddy Just released v3.0.3 with a fix. It's released on Firefox and Chrome. Let me know if it fixes your issue.

@sorinpav Feel free to try the new version as well, but I suspect the issue you're having is caused by something else.

@a-c-sreedhar-reddy
Copy link

a-c-sreedhar-reddy commented Jan 12, 2022

It worked with 3.0.3. Thanks @Methuselah96 ✌🏻

@robmcm
Copy link

robmcm commented Jan 12, 2022

Thanks @Methuselah96 3.0.3 also fixes an issue we were having with accessing the observable manually using symbol-observable. Appreciate the quick turnaround!

@Methuselah96
Copy link
Member

@markerikson To conclude our earlier conversation, it turns out RxJS stopped using a polyfill for Symbol.observable as well which makes me agree that Redux should not be using symbol-observable since the original intent of making the Redux store observable was to make the store interop with RxJS.

The solution of making sure to run polyfills first doesn't work for the extension since the Chrome extension code runs before any user code. I've made it so that the DevTools doesn't resolve Symbol.observable until the user creates a store and it will produce a console warning if Symbol.observable doesn't match between Redux and the DevTools. If a user gets that warning it means that they polyfilled Symbol.observable after importing Redux and before creating a store.

@sorinpav
Copy link

@Methuselah96 It appears that the 3.0.3 version did not sort our issue. I've tried both the "-next" and the official Redux-devtools extensions. v3.0.3. They don't seem to work for us. Should I start a new issue, or should we reopen this one? Also, could you please re-iterate what details you would need from our config?

@Methuselah96
Copy link
Member

@sorinpav I think starting a new issue would be preferable since it's a different root issue. If you could specify how you're creating your store similarly to how it was done here and here that would be great. Thanks!

@markerikson
Copy link
Contributor

@Methuselah96 gotcha. Really appreciate your detective work and digging here!

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

Successfully merging a pull request may close this issue.

6 participants