Support for latest 6.x version of justinrainbow/json-schema package

This was not possible/permitted by Drupal core, but apparently 11.2 changed that, nice: 📌 Allow 6.x version of justinrainbow/json-schema Active .

Duplicate of 💬 Support justinrainbow/json-schema version 6 in XB. Active though … 😅 A simple search for immediately reveals that. Next time, please verify an issue doesn't already exist. 🙏 Here, closing that one as a duplicate, since this one already has an MR.

Also, all new MRs must target the 1.x branch. 🙏

This should be safely backportable to 0.x because it only loosens Composer constraints.

Yes, Wim, I agree to search for issues to avoid duplication.I tried earlier, but I noticed that I might have a category filter set to (Bug/task), which is why it did not appeared.

No problem!

Please note that this MR is not mergeable right now, as described in #5. Please update it.

That … was not quite right. There's lots of pointless noise in there. Fixing.

Let's see what tests say (and let's verify they are indeed using version 6).

That passes, but … is actually testing using 5.3, not 6:

- Installing justinrainbow/json-schema (5.3.0): Extracting archive

Please make the necessary changes to the CI logic — or perhaps even flat-out require version 6? — to ensure we're actually testing against that version. Currently, the green CI results are quite menaingless 😅

If we do this, we should try requiring https://github.com/jsonrainbow/json-schema/releases/tag/6.3.0 because that includes https://github.com/jsonrainbow/json-schema/pull/800, which has long plagued us.

Figured out the root cause over at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... + https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... — root cause is #3538439 — see #3538439-13: Running tests locally is difficult to match with the CI → .

Pushed commits to hopefully solve this 🤞

wim leers → credited larowlan → .

@larowlan indicated in ✨ [PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed — where he added UriFormatAwareFormatConstraint — that that would become obsolete once we require version 6. Let's find out! :D

Assuming this passes: RTBC.

Didn't work: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

It didn't work because of one silly reason: entity:node/1 (the source value) evaluates/resolves to /node/1, and the hero component in the test has the cta1href property defined as:

    cta1href:
      type: string
      format: uri
      title: CTA 1 link
      examples:
        - https://example.com

We just need to change format: uri (which accepts only absolute URIs) to format: uri-reference (which also allows relative ones).

This was a simple oversight in ✨ [PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed .

That passed!

Note that this makes Experience Builder a bit more strict. If an SDC or code component wants to allow relative URLs, it MUST use format: uri-reference (which then also supports absolute URLs). If it wants to allow ONLY absolute URLs, it MUST use format: uri.

This is not XB's decision; it's defined by JSON Schema: see https://opis.io/json-schema/2.x/formats.html#uri vs https://opis.io/json-schema/2.x/formats.html#uri-reference, which is what the SDC architecture is based upon.

And note that the code component editor UI has explicitly supported selecting either since its introduction in ✨ Add support for creating links within code components Active :

(But it could be a little clearer — created 📌 Code component editor UI "Link" prop type allows picking "full" vs "relative path", but is misleading Active for that.)

This is important to get right prior to 1.0, but may impact some existing sites/users — so let's do a change record.

After some tricky battling/balancing, I ended up dedicating my-hero to test format: uri-reference (mostly because ✨ [PP-1] Make link widget autocomplete work (for uri and uri-reference props) Postponed specifically uses this to test entity:node/5 working correctly) and my-cta to test format: uri, with comments pointing to each other.

Also tricky: Drupal core's uri field type (and data type) only allows absolute URLs, so \Drupal\xb_test_storage_prop_shape_alter\Hook\XbTestStoragePropShapeAlterHooks::storagePropShapeAlter() MUST specifically only target format: uri, not format: uri-reference. Related: 🐛 `format: uri` and `format: uri-reference` shape matching is too coarse Active .

Change record created: https://www.drupal.org/node/3542915 →

Also: this blocks 📌 Decouple image (URI) shape matching from specific image file types/extensions Active .

We can't backport this, because as this MR shows, there's a whole slew of changes necessary to make this possible.

+1 RTBC !1428

I tested this by:

> ddev composer update -W
[...]
  - Upgrading justinrainbow/json-schema (5.3.0 => 6.4.2): Extracting archive
  - Upgrading composer/composer (2.8.6 => 2.8.11): Extracting archive

Verified ddev composer -d /var/www/html/web/modules/contrib/experience_builder run install-dev-deps still works.

Tested that one random test from our http internal api that does json validation still pass, + no random errors when running phpstan (which has been always tricky with versions too).

On my existing site I was force to drush cr --yes per

JsonSchema\Exception\InvalidArgumentException: Unknown constraint format in JsonSchema\Constraints\Factory->setConstraintClass() (line 164 of /var/www/html/vendor/justinrainbow/json-schema/src/JsonSchema/Constraints/Factory.php).

(which I'm guessing is related to the previously unsupported regex)

Thanks for the thorough review!

On my existing site I was forced to drush cr --yes

That's a very good remark — I'll double-check core's body of updates + tests to see if we need to trigger a container rebuild using updates.

(IIRC an empty post-update hook achieves that. But it's been years since I've had to do that. Will verify.)

P.S.: this reminded me we also need to do https://www.drupal.org/node/3473563 → — we aren't doing that yet in our https://git.drupalcode.org/project/experience_builder/-/blob/1.x/experie....

wim leers → changed the visibility of the branch 3530351-shape-it-up to hidden.

Found core example for #25: system_post_update_remove_rss_cdata_subscriber(). Introduced in 🐛 RssResponseCdata filtering out common html tags Active . No update path test.

Matched that.

After a final round of manual testing in the UI with both images and the entity (node) autocomplete surfaced no problems: let's get this in!

This gets us properly on full Drupal 11.2 support, and fixes some of the loose ends from 📌 Runnings tests locally is difficult to match with the CI Active . 👍