Skip to content

Immutable v4 compatibility #61

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

Merged
merged 6 commits into from
Mar 14, 2017
Merged

Conversation

Velenir
Copy link
Contributor

@Velenir Velenir commented Mar 14, 2017

Closes #46

Also I would like to propose separating immutable dependency into peerDependencies and devDependencies:

"peerDependencies": {
  "immutable": "^3.8.1 || ^4.0.0-rc.1"
},
"devDependencies": {
  "immutable": "^3.8.1 || ^4.0.0-rc.1"
}

That way if a user's project at large uses immutable@^4, redux-immutable won't try to pull in immutable@3.8.1 as a local dependency, but rather import from the project. Avoids bloating up bundle size.

Conditional devDependencies avoids npm warnings when switching between immutable versions in development. 3.8.1 is installed by default.

@gajus
Copy link
Owner

gajus commented Mar 14, 2017

This looks good. Is is ready to be merged?

@Velenir
Copy link
Contributor Author

Velenir commented Mar 14, 2017

If you agree that immutable dependency should be separated as I proposed, then I'll quickly add a commit with that. Or would you prefer a separate PR?

@gajus
Copy link
Owner

gajus commented Mar 14, 2017

If you agree that immutable dependency should be separated as I proposed, then I'll quickly add a commit with that.

👍 go for it, this PR is fine.

@Velenir
Copy link
Contributor Author

Velenir commented Mar 14, 2017

Good to go 😄

@gajus gajus merged commit db26f25 into gajus:master Mar 14, 2017
@gajus
Copy link
Owner

gajus commented Mar 14, 2017

Thank you

@Velenir
Copy link
Contributor Author

Velenir commented Mar 14, 2017

Thank you for such a useful module.

@jeffchan
Copy link

@gajus Thanks for the package! We upgraded to Immutable v4 and noticed that this change, while merged, isn't published to NPM yet. Is it possible to publish it?

@jeffchan
Copy link

Nevermind my comment. It is indeed already published as v4.

@andrewmclagan
Copy link

Would people consider bumping this to

"peerDependencies": {
  "immutable": "^3.8.1 || ^4.0.0-rc.9"
},
"devDependencies": {
  "immutable": "^3.8.1 || ^4.0.0-rc.9"
}

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.

4 participants