WordPress.org

Make WordPress Core

Opened 20 months ago

Last modified 20 months ago

#36946 new defect (bug)

Provide `id` properties on core objects

Reported by: jeremyfelt Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: General Keywords: needs-patch
Focuses: Cc:

Description

In #36717, WP_Site and WP_Network will get magic getters/setters so that id and other more properly named properties can be used.

We should standardize on id rather than ID for core objects where it makes sense.

Current primary IDs for objects are:

  • WP_Comment -> comment_ID
  • WP_Post -> ID
  • WP_Term -> term_id
  • WP_User -> ID
  • WP_Widget -> id
  • WP_Screen -> id

WP_User is somewhat unique because the property was id and then deprecated in [18504] in favor of ID. We should be able to un-deprecate this.

Change History (7)

#1 follow-up: @DrewAPicture
20 months ago

My vote would be on standardizing on ID as it's actually the abbreviation for identifier and is what we (correctly) use all over the place in inline docs.

'id' makes me think of Freud. :-)

#2 in reply to: ↑ 1 ; follow-ups: @jeremyfelt
20 months ago

Replying to DrewAPicture:

My vote would be on standardizing on ID as it's actually the abbreviation for identifier and is what we (correctly) use all over the place in inline docs.

'id' makes me think of Freud. :-)

Here's the related discussion from today's core meeting. Hopefully to sway you a bit. :)

Also worth noting that the WP REST API team is using id in responses for core objects.

#3 in reply to: ↑ 2 ; follow-up: @DrewAPicture
20 months ago

Replying to jeremyfelt:

Replying to DrewAPicture:

My vote would be on standardizing on ID as it's actually the abbreviation for identifier and is what we (correctly) use all over the place in inline docs.

'id' makes me think of Freud. :-)

Here's the related discussion from today's core meeting. Hopefully to sway you a bit. :)

Also worth noting that the WP REST API team is using id in responses for core objects.

Interesting. Of course it also doesn't help that we're sorely inconsistent at the db level either:

  • commentmeta: meta_id, comment_id
  • comments: comment_ID, comment_post_ID, user_id
  • options: option_id
  • postmeta: meta_id, post_id
  • posts: ID
  • termmeta: meta_id, term_id
  • terms: term_id
  • term_relationships: object_id, term_taxonomy_id
  • term_taxonomy: term_taxonomy_id, term_id
  • usermeta: umeta_id, user_id
  • users: ID

Pretty much the only thing that's consistent there is that when it's 'ID' alone, it's 'ID'. Yuck.

All that aside, per the linked discussion, I really don't think a deprecated notice or a _doing_it_wrong() is a good idea, considering our history and the fact that $user->ID and $post->ID are used all over the WordPress ecosystem.

#4 in reply to: ↑ 3 ; follow-up: @dd32
20 months ago

Replying to DrewAPicture:

All that aside, per the linked discussion, I really don't think a deprecated notice or a _doing_it_wrong() is a good idea, considering our history and the fact that $user->ID and $post->ID are used all over the WordPress ecosystem.

I really agree with this, I don't think we should be firing deprecated notices for everything but rather just have the different-case'd variants "just work". There's zero harm in using ->ID over ->id, it's not like we're ever going to remove the ID versions.

#5 in reply to: ↑ 4 @jeremyfelt
20 months ago

Replying to dd32:

Replying to DrewAPicture:

All that aside, per the linked discussion, I really don't think a deprecated notice or a _doing_it_wrong() is a good idea, considering our history and the fact that $user->ID and $post->ID are used all over the WordPress ecosystem.

I really agree with this, I don't think we should be firing deprecated notices for everything but rather just have the different-case'd variants "just work". There's zero harm in using ->ID over ->id, it's not like we're ever going to remove the ID versions.

Agreed. I don't think we need to deprecate anything. Just un-deprecate WP_User::id. :)

#6 @jb510
20 months ago

I was going to +1 ID, but the slack log convinced me otherwise...

Is it worth discussing function and hook names at the same time? get_the_ID(), the_ID(), get_author_user_ids, get_current_blog_id().

Seems like it's only the oldest of functions using ID.

#7 in reply to: ↑ 2 @rmccue
20 months ago

Replying to jeremyfelt:

Also worth noting that the WP REST API team is using id in responses for core objects.

We switched away from uppercase ID because it's inconsistent with every other field. We have properties like display_name, post_title, etc, but ID doesn't match this. It looks especially weird when you have things like site_ID. Plus: every other abbreviation is lowercase, except for ID. Under the current system, you could end up with http_url_ID, which makes 0 sense.

While @DrewAPicture is correct that "ID" is the correct casing for the English abbreviation, we're not using regular English sentences, we're using snake_case, which is all lowercase.

Replying to jb510:

Is it worth discussing function and hook names at the same time? get_the_ID(), the_ID(), get_author_user_ids, get_current_blog_id().

PHP function names are case-insensitive, so I think it makes sense to re-case all these at the same time, since it won't break anything. Docs are the main thing that will need updating.

Note: See TracTickets for help on using tickets.