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

Utils: Deprecate buildTermTree function #7976

Merged
merged 1 commit into from
Jul 16, 2018
Merged

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Jul 16, 2018

Part of #3955.

This PR duplicates buildTermsTree function and puts it closer to the usage. We took this approach to make it possible to get rid of utils module at all. In the long term, we might want to seek to remove duplication by moving all the logic that build terms tree closer to the data layer which is where it should really happen.

As part of this PR we completely deprecate (remove) wp.utils.buildTermsTree.

@gziolo gziolo added the npm Packages Related to npm packages label Jul 16, 2018
@gziolo gziolo added this to the 3.3 milestone Jul 16, 2018
@gziolo gziolo self-assigned this Jul 16, 2018
@gziolo gziolo requested a review from youknowriad July 16, 2018 09:53
Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this change, this function is too specific to be really valuable as an exposed API

/**
* Internal dependencies
*/
import { buildTermsTree } from '../terms';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need the test twice?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case it diverges, it might make sense.

@gziolo gziolo force-pushed the update/deprecate-utils-module branch from 7c0247a to 53e8f1c Compare July 16, 2018 10:13
@gziolo gziolo merged commit 581522d into master Jul 16, 2018
@gziolo gziolo deleted the update/deprecate-utils-module branch July 16, 2018 10:25
@gziolo gziolo added the [Type] Breaking Change For PRs that introduce a change that will break existing functionality label Jul 16, 2018
@chrisvanpatten
Copy link
Contributor

I'm curious why this decision was made?

Right now we offer a public TreeSelect component, and the ability to fetch terms (or hierarchical post types) through the data store, but without this — the function that actually bridges the two together — users have to reimplement this logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
npm Packages Related to npm packages [Type] Breaking Change For PRs that introduce a change that will break existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants