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

Breaking changes introduced in minor version #135

Closed
Jameskmonger opened this issue Jan 29, 2018 · 12 comments · Fixed by #136
Closed

Breaking changes introduced in minor version #135

Jameskmonger opened this issue Jan 29, 2018 · 12 comments · Fixed by #136

Comments

@Jameskmonger
Copy link

In redux-mock-store@1.4.0, the schema of the export is like this:

export = {
    default: function configureStore() { ... }
}

In redux-mock-store@1.5.0, the schema of the export is like this:

export = function configureStore() { ... }

The change between versions 1.4.0 and 1.5.0 is a minor version change. Breaking changes should not have been introduced in this version.

This package has almost 30k downloads per day, so to introduce breaking changes in a minor version has quite a large impact.

The best plan of action would be to mark version 1.5.0 and 1.5.1 as deprecated with:

npm deprecate redux-mock-store@1.5.0 "breaking changes in minor version"
npm deprecate redux-mock-store@1.5.1 "breaking changes in minor version"

and to republish the current version as 2.0.0.

@hisuwh
Copy link

hisuwh commented Jan 29, 2018

Why change this anyway?

@Jameskmonger
Copy link
Author

@hisuwh

Why change this anyway?

I think this is due to #132, but that's not marked as a breaking change in the release notes...

@dmitry-zaets
Copy link
Collaborator

Sorry for the inconvenience.
I've deprecated both 1.5.0 and 1.5.1
We would test new version better to avoid breaking change or bugs.

@Jameskmonger
Copy link
Author

It may be worth adding some unit tests around importing (probably more of an integration test), but something like this:

Expect(reduxMockStore)
    .toBeAFunction()
    .withName("configureStore");

Expect(reduxMockStore.default)
    .toBeAFunction()
    .withName("configureStore");

(pseudocode)

@csi-lk
Copy link

csi-lk commented Feb 5, 2018

Hmm, I've got my build failing as this dependency is now deprecated, as far as I can tell there is no upgrade target?

Would be good it could be republished at 2.0.0 as @Jameskmonger suggested

@alasdairhurst
Copy link

alasdairhurst commented Feb 6, 2018

Not really much point releasing a 2.0.0 if the breaking change was unintentional/unnecessary. This issue at the moment anyone will still pick up the deprecated version with breaking changes as latest, so a minor/patch version which reverts the breaking change should ideally be released as latest so anyone depending on ^ will pick it up without problems.

@reesscot
Copy link

Agree with @alasdairhurst on this one, @dmitry-zaets can we please get the change reverted and republished as the next minor release number, 1.6.0 or patch version 1.5.2? Still getting deprecated messages in NPM.

@Jameskmonger
Copy link
Author

It should be released as a patch version ideally as it is patching a break.

@thomashagstrom
Copy link

thomashagstrom commented Feb 27, 2018

Do mind the TypeScript mappings (@typings/redux-mock-store), that need to be synced with these changes.

@innerdaze
Copy link

Did this ever happen? It's now April

@dmitry-zaets
Copy link
Collaborator

Sorry for the late response and the inconvenience caused by the breaking changes that we introduced.
We didn't expect that it would actually cause the breaking change.
Version 1.4.0. was republished as 1.5.3 in order to fix the mess around the breaking changes.

@mrichmond
Copy link

Thank you... a bit concerning that it took 5 months to release the patch though...

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 a pull request may close this issue.

9 participants