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

Immutable 4 is breaking change - release React 16 version that still works with immutable 3 #3493

Closed
ntucker opened this issue Oct 9, 2017 · 11 comments

Comments

@ntucker
Copy link

ntucker commented Oct 9, 2017

Are you submitting a bug report or a feature request?

Following semver, changing immutable to 4 should probably be a major release, as 3 would no longer be supported. However, I think many would like React 16 compatibility before they upgrade to the not-yet-released immutable 4.

What is the current behavior?

7.1 requires Immutable 4 and 3 does not work

What is the expected behavior?

Version 7 should not require a breaking library bump of immutable.

Sandbox Link

What's your environment?

Other information

@erikras
Copy link
Member

erikras commented Oct 9, 2017

How might this library detect what version of Immutable you are using?

@erikras
Copy link
Member

erikras commented Oct 9, 2017

What, exactly, is broken when using redux-form@7.1.0 with immutable@3?

The immutable example seems to be working normally.

@hsz
Copy link
Contributor

hsz commented Oct 9, 2017

@erikras I confirm this issue.

When I use redux-form@7.1.0 with immutable@3.8.2 I get:

  TypeError: (0 , _immutable.isCollection) is not a function
  
  - index.js:42 getIn
    [my-project]/[redux-form]/lib/structure/immutable/index.js:42:40
  
  - createFormValueSelector.js:22 nonNullGetFormState
    [my-project]/[redux-form]/lib/createFormValueSelector.js:22:14
  
  - createFormValueSelector.js:31 
    [my-project]/[redux-form]/lib/createFormValueSelector.js:31:13
  
  - index.js?:1 Function.mapStateToProps [as mapToProps]
    webpack:///./src/components/MyComponent/index.js?:1:5775
  
  - wrapMapToProps.js:54 mapToPropsProxy
    [my-project]/[react-redux]/lib/connect/wrapMapToProps.js:54:92
  
  - wrapMapToProps.js:63 Function.detectFactoryAndVerify
    [my-project]/[react-redux]/lib/connect/wrapMapToProps.js:63:19
  
  - wrapMapToProps.js:54 mapToPropsProxy
    [my-project]/[react-redux]/lib/connect/wrapMapToProps.js:54:46
  
  - selectorFactory.js:37 handleFirstCall
    [my-project]/[react-redux]/lib/connect/selectorFactory.js:37:18
  
  - selectorFactory.js:85 pureFinalPropsSelector
    [my-project]/[react-redux]/lib/connect/selectorFactory.js:85:81
  
  - connectAdvanced.js:43 Object.runComponentSelector [as run]
    [my-project]/[react-redux]/lib/components/connectAdvanced.js:43:25
  
  - connectAdvanced.js:195 Connect.initSelector
    [my-project]/[react-redux]/lib/components/connectAdvanced.js:195:23
  
  - connectAdvanced.js:136 new Connect
    [my-project]/[react-redux]/lib/components/connectAdvanced.js:136:15
  
  - ReactCompositeComponent.js:294 
    [my-project]/[react-dom]/lib/ReactCompositeComponent.js:294:18
  
  - ReactCompositeComponent.js:75 measureLifeCyclePerf
    [my-project]/[react-dom]/lib/ReactCompositeComponent.js:75:12
  
  - ReactCompositeComponent.js:293 ReactCompositeComponentWrapper._constructComp    onentWithoutOwner
    [my-project]/[react-dom]/lib/ReactCompositeComponent.js:293:16
  
  - ReactCompositeComponent.js:279 ReactCompositeComponentWrapper._constructComp    onent
    [my-project]/[react-dom]/lib/ReactCompositeComponent.js:279:21

With reverting to redux-form@7.0.4 everything works fine.

@ntucker
Copy link
Author

ntucker commented Oct 9, 2017

My suggestion is to have different versions of this library for each immutable. Currently I imagine most people are on 3, so keep supporting that, but have the next versions (8+) support 4 only.

@tyscorp
Copy link
Contributor

tyscorp commented Oct 9, 2017

Can confirm that v7.1.0 is broken with both immutable@3.x and immutable@4.x.

@bdwain
Copy link
Contributor

bdwain commented Oct 9, 2017

to support both in the same release you would just need to check whether a feature exists before trying to call it. For example, Immutable 4 renamed Iterable to Collection and isIterable to isCollection. You could check to see if Collection is defined before using it, and if not, use Iterable. No need to check for versions explicitly.

@NadavGab
Copy link

NadavGab commented Oct 10, 2017

I got the same issue: TypeError: (0 , _immutable.isCollection) is not a function
I am using redux-form@7.0.4 with immutable@3.8.1.
also tried redux-form@7.1.0 with immutable@3.8.2, redux-form@7.0.4 with immutable@3.8.2 and
redux-form@7.1.0 with immutable@4.0.0-rc.7

Edit:
Fixed - the problem fixed after upgrade the npm version from 3.X to 5

@couturecraigj
Copy link

I think redux-form is already quite big to have added checks, so this seems to be something we should avoid. There should be two different versions 7.1.0 should be immutable 3 and 8 should be with 4. It is a breaking change even if it is a dependency.

As a side note: Version 4 is not even official yet we are still looking at Release Candidates. It should be ignored until it has been made official. When performing an npm update immutable 4.0.0rc1-7 will be passed. Just my two cents.

@erikras
Copy link
Member

erikras commented Oct 10, 2017

Oops. Git branch mistake... I've rolled back to Immutable v3. 03c37d7

@erikras
Copy link
Member

erikras commented Oct 10, 2017

Published fix in v7.1.1. We won't move to immutable@4 until it's released. Please accept my sincere apology for this mixup.

It was caused by trying to fix a Flow error. immutable-js/immutable-js#1308

@lock
Copy link

lock bot commented Oct 10, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Oct 10, 2018
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

7 participants