The JSON API core patch at #2843147-51: Add JSON:API to core as a stable module is failing tests because of this deprecation error in many dozens of the tests:

Remaining deprecation notices (1)

  1x: \Drupal\Core\Routing\Enhancer\RouteEnhancerInterface is deprecated in Drupal 8.5.0 and will be removed before Drupal 9.0.0. Instead, you should use \Drupal\Core\Routing\EnhancerInterface. See https://www.drupal.org/node/2894934

See the corresponding CR at https://www.drupal.org/node/2894934.

The tricky thing is that we can't fix this in JSON API 8.x-1.x because \Drupal\Core\Routing\EnhancerInterface only exists in 8.5. Only JSON API 8.x-2.x will require >=8.5.0.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Status: Active » Needs review
FileSize
811 bytes
Wim Leers’s picture

FileSize
832 bytes
1.56 KB

I forgot one.

gabesullice’s picture

This will break other routes that use the page parameter because applies will never be called again. See: #2961773-7: Update ParamEnhancers to not use deprecated interface to prevent unintentional interactions with requests

The attached fixes that.

gabesullice’s picture

Wim Leers’s picture

  1. +++ b/src/Routing/Routes.php
    @@ -189,6 +189,20 @@ class Routes implements ContainerInjectionInterface {
    +  public static function isJsonApiRequest(array $defaults) {
    +    return isset($defaults[RouteObjectInterface::CONTROLLER_NAME])
    +      && $defaults[RouteObjectInterface::CONTROLLER_NAME] === static::FRONT_CONTROLLER;
    

    Checking

    '_is_jsonapi' => TRUE,
    

    would be simpler/nicer? It'd also allow custom modules to expand JSON API without being forced to use the same front controller. (Not recommended, but some custom projects may still want to do this.)

  2. +++ b/src/Routing/Routes.php
    @@ -189,6 +189,20 @@ class Routes implements ContainerInjectionInterface {
    +  public static function isJsonApiRequest(array $defaults) {
    

    Why public?

gabesullice’s picture

It'd also allow custom modules to expand JSON API without being forced to use the same front controller. (Not recommended, but some custom projects may still want to do this.)

I think the original intent for this way of checking was to prevent expanding JSON API in unsupported ways.

Why public?

So that it can be used from both route enhancers. I suppose a trait could work, but that seemed like overkill. What do you think?

gabesullice’s picture

+++ b/src/Routing/JsonApiParamEnhancer.php
@@ -48,18 +47,14 @@ class JsonApiParamEnhancer implements RouteEnhancerInterface {
-    return $route->getDefault(RouteObjectInterface::CONTROLLER_NAME) == Routes::FRONT_CONTROLLER;

This is the code that the static method replaced.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

You're right, #6.1 is misguided because it's just moving existing logic into a separate method.

#6.2 makes sense too, I misread it first, my bad!

gabesullice’s picture

Needed a reroll, committing if tests pass.

  • gabesullice committed 7d07d79 on 8.x-2.x
    Issue #2957271 by gabesullice, Wim Leers: [>=8.5] Fix...
gabesullice’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
1002 bytes

Fixed the two CS violations.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Kristen Pol’s picture

Issue tags: +Drupal 9 compatibility

Per a Slack discussion with Gábor Hojtsy regarding usage of D9 tags (Drupal 9, Drupal 9 compatibility, Drupal 9 readiness, etc.), "Drupal 9 compatibility" should be used for contributed projects that need updating and "Drupal 9" was the old tag for D8 issues before the D9 branch was ready. Doing tag cleanup here based on that discussion.