feat: legacy context usages

[cdoublev]

This is a raw PR to fix #101 . I did not take care of types, tests, or whatever.

You can test upgrading/downgrading from react-redux@v5.1.0 / to react-redux@v6.0.0 with the redux-first-router branch of this repository as a simple boilerplate and run npm run start:client or npm run start:dev or npm run start.

[mungojam]

It looks like this changes the API by introducing a new location prop to both Link components. Is that a new feature?

[cdoublev]

You're right that location should not be mixed in the public interface. It is replacing the pathname prop of <NavLink>, which was also not meant to be defined by the user. How would you hide them?

[ScriptedAlchemy]

this one seems a little more robust: https://github.com/faceyspacey/redux-first-router-link/pull/113/files

is there anything you'd want o to add to that PR?

[cdoublev]

Sure, you can merge it, in my opinion. But @mungojam made a relevant observation about giving pathname/location to <Link>/<NavLink> from state via props. Chances that a user would need to pass down a prop with those names are small but... who knows?

My opinion is to merge it because this risk already exist by mapping dispatch and pathname to props, and the only workarounds I found are:

1. prefixing ownProps properties with an underscore but the risk would still remain (but smaller)
2. accessing to store and storeState via static contextType = ReactReduxContext and this.context, but then you would have to implement shouldComponentUpdate and I thought it was not worth it.

[mungojam]

I wouldn't worry about my point if you don't think it's likely to affect people.

I'm not familiar enough with the library to know what all the parameters do, so it was more that I was checking if the API was intentionally changed in the PR or if that could be done in a separate release if it was an unrelated improvement

[ScriptedAlchemy]

Okay guys. Christmas time and I’m mobile. Merge this and automate release, yes or no?

Do we cater for legacy or is this a breaking change deserving of a major release?

[circlingthesun]

This shouldn't break legacy code as it just uses the regular react-redux API. My vote is for merge.

[ScriptedAlchemy]

As you wish! If automated releases don’t work I’ll remotely fix that. But I know I’ve fixed most repos now

[ScriptedAlchemy]

Friendly reminder to use semantic-release commit conventions or use commitizen.

@cdoublev

[ScriptedAlchemy]

Releasing as a feat minor bump to be safe. Don’t want the community blowing up if the next patch version automatically installs.

[ScriptedAlchemy]

:tada: This PR is included in version 2.1.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[mungojam]

This shouldn't break legacy code as it just uses the regular react-redux API. My vote is for merge.

My mistake, I'm not used to functional components that have state and/or prop-types and/or flow and I can see now that location was added to the state, not the props of the component

I look forward to using this new release and removing my horrid new-old context adaptor :)

[cdoublev]

this one seems a little more robust: https://github.com/faceyspacey/redux-first-router-link/ pull/113/files

is there anything you'd want o to add to that PR?

I was expecting #113 being merged instead of this PR, which was raw, as indicated in its first comment. I'm confused.

[GuillaumeCisco]

I was expecting #113 too regarding the discussion (:

[ScriptedAlchemy]

Whoooops Okay, i was on my phone and got confused. Can @GuillaumeCisco please pull from master and resolve the conflicts, I’m at work right now but ill totally merge in your PR. Glad i did a version bump now! Haha

So sorry guys, the holiday season has be in worse attention span than usual.