close
The Wayback Machine - https://web.archive.org/web/20200916175519/https://github.com/gajus/redux-immutable/pull/61
Skip to content

/ redux-immutable

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 v4 compatibility #61

Merged
merged 6 commits into from Mar 14, 2017
Merged

Immutable v4 compatibility #61

merged 6 commits into from Mar 14, 2017

Conversation

@Velenir
Copy link
Contributor

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.
Velenir added 4 commits Mar 14, 2017
@Velenir
fix: pass type check for Record in Immutable v4
010cf83
@Velenir
fix: extracting keys from Record in Immutable v4
f1b7c67
@Velenir
refactor: change warning to better reflect Immutable v4 types
88d33d4
@Velenir
docs: better reflect both Immutable v3 and v4 types
Loading status checks…
7c3a01f
@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 added 2 commits Mar 14, 2017
@Velenir
docs: replace Immutable.Iterable with Immutable.Collection
4873140
@Velenir
build: change dependency type for immutable and extend version supported
Loading status checks…
a511528
@Velenir
Copy link
Contributor Author

Velenir commented Mar 14, 2017

Good to go 😄
@gajus gajus merged commit db26f25 into gajus:master Mar 14, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@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

jeffchan commented Feb 28, 2018

@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

jeffchan commented Feb 28, 2018

Nevermind my comment. It is indeed already published as v4.
@andrewmclagan
Copy link

andrewmclagan commented Jun 5, 2018

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
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@Velenir @gajus @jeffchan @andrewmclagan
You can’t perform that action at this time.