[wp-trac] [WordPress Trac] #45265: REST API: register_rest_route should warn when used improperly.

[WordPress Trac]

#45265: REST API: register_rest_route should warn when used improperly.
--------------------------------------+------------------------
 Reporter:  kraftbj                   |       Owner:  desrosj
     Type:  defect (bug)              |      Status:  reviewing
 Priority:  normal                    |   Milestone:  5.1
Component:  REST API                  |     Version:
 Severity:  normal                    |  Resolution:
 Keywords:  has-patch needs-dev-note  |     Focuses:  rest-api
--------------------------------------+------------------------
Changes (by desrosj):

 * keywords:  has-patch => has-patch needs-dev-note
 * owner:  SergeyBiryukov => desrosj


Comment:

 Thanks @kraftbj! This was
 [https://wordpress.slack.com/archives/C02RQC26G/p1547142171152900
 discussed today's bug scrub] and
 [https://wordpress.slack.com/archives/C02RQC26G/p1547149412173300 then
 again after] in #core-restapi. Here is a summary of the discussion:

 - This `_doing_it_wrong()` notice should not be accompanied by a `return`.
 That could be a pretty bad breaking change for some setups. The other
 notices already in the `register_rest_route()` function have been in place
 and enforced with a `return` since the REST API was merged into Core.
 - I moved the new notice below the pre-existing ones. If the preexisting
 ones are already being triggered on a site, they should continue to be the
 notice that appears until fixed.

 When I first started testing this, I realized that 66 of the test methods
 in Core were failing. This is because the methods do not call
 `register_rest_route()` on the action hook. It was decided that `if ( !
 did_action( 'rest_api_init' ) && ! doing_action( 'rest_api_init' ) )` was
 a better approach and would cut down on the number of notices. This change
 brought the number of failures down to 11, and those are fixed in
 [attachment:"45265.2.diff"] by including `@expectedIncorrectUsage`
 notations.

 I am marking this as `needs-dev-note` mainly because of the potential for
 these failures to start happening in plugin and theme test suites that
 call `register-rest_route()` in a similar manner. But, this is also a good
 opportunity to educate on why this is the correct way to register routes.
 A few reasons that were discussed:

 - [https://wordpress.slack.com/archives/C02RQC26G/p1547151230185800
 Performance implications]: One call to `register_rest_route` causes all
 plugins to register their routes.
 - If called incorrectly before plugins are able to register their
 `rest_api_init` hooks (say in an `mu-plugin` file), then the hook will not
 fire a second time and those endpoints will never be registered.

 I will prepare a rough draft dev note. If it can be published in the next
 5-7 days, I think that this could still go into 5.1.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/45265#comment:4>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform