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

Remove 4.4 deprecated features. #11927

Merged
merged 7 commits into from Nov 15, 2018

Conversation

Projects
None yet
4 participants
@youknowriad
Contributor

youknowriad commented Nov 15, 2018

No description provided.

@youknowriad youknowriad added this to the 4.4 milestone Nov 15, 2018

@youknowriad youknowriad self-assigned this Nov 15, 2018

@youknowriad youknowriad requested a review from WordPress/gutenberg-core Nov 15, 2018

@youknowriad youknowriad force-pushed the update/remove-deprecated-features branch from d321118 to 5efd46a Nov 15, 2018

@youknowriad

This comment has been minimized.

Contributor

youknowriad commented Nov 15, 2018

@aduth you beat me to it :)

@aduth

This comment has been minimized.

Member

aduth commented Nov 15, 2018

Caught some more in 3adb4f2

### Breaking Changes
- `registry.registerReducer` has been deprecated. Use `registry.registerStore` instead.

This comment has been minimized.

@gziolo

gziolo Nov 15, 2018

Member

Should be:

has been removed

reducer = options.reducer;
store = createReduxStore( reducer, key, registry );
}
let selectors = {};

This comment has been minimized.

@aduth

aduth Nov 15, 2018

Member

These never defaulted to objects previously, do they need to? vs. let selectors, resolvers, actions;

This comment has been minimized.

@youknowriad

youknowriad Nov 15, 2018

Contributor

Yes, probably not impactful, I don't want to wait for the tests once more :P

This comment has been minimized.

@aduth

aduth Nov 15, 2018

Member

It could be good to enforce one way or the other by tests. I think it has an impact on what's returned from select( 'store-not-exists' ). What would you expect the return value to be?

In master, it's undefined. In this branch, it's now {}.

Impactful 😬

This comment has been minimized.

@youknowriad

youknowriad Nov 15, 2018

Contributor

really? I don't expect this change to impact that though :)

This comment has been minimized.

@youknowriad

youknowriad Nov 15, 2018

Contributor

It is still returning undefined for me.

This comment has been minimized.

@aduth

aduth Nov 15, 2018

Member

Oh, I guess it's:

wp.data.registerStore( 'store-no-selectors', { reducer: () => null } );
wp.data.select( 'store-no-selectors' );

Master: undefined
This branch: {}

This comment has been minimized.

@aduth

aduth Nov 15, 2018

Member

I was going to add unit tests, but I think in retrospect it may have been improved by the {} change being the expected behavior. Let's deal with it separately.

This comment has been minimized.

@aduth

This comment has been minimized.

@coderkevin

coderkevin Nov 16, 2018

Contributor

I don't have a strong opinion either way on this. {} makes sense to me.

@aduth

aduth approved these changes Nov 15, 2018

@aduth

aduth approved these changes Nov 15, 2018

@youknowriad youknowriad merged commit 76c629f into master Nov 15, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@youknowriad youknowriad deleted the update/remove-deprecated-features branch Nov 15, 2018

grey-rsi pushed a commit to OnTheGoSystems/gutenberg that referenced this pull request Nov 22, 2018

Remove 4.4 deprecated features. (WordPress#11927)
* Remove 4.4 deprecated features

* Remove deprecated dependency and update changelogs

* Fix unit tests

* Framework: Update changed files from install, docs build

* Data: Remove granular register functions from public API

* Fix typos

* Default to undefined selectors/actions/resolvers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment