Problem/Motivation

https://github.com/focus-trap/tabbable/releases/tag/v6.0.0

CommentFileSizeAuthor
#2 3306167-2.patch65.86 KBlauriii
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lauriii created an issue. See original summary.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Active » Needs review
FileSize
3306167-2.patch65.86 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

The main breaking change in tabbable 6.0 is dropping support for IE11, which is fine by us.

There is also a breaking change in the displayCheck option, which we don't appear to use at all.

Therefore assuming the bot agrees, this is RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/core.libraries.yml
--- a/core/package.json
+++ b/core/package.json

+++ b/core/package.json
@@ -104,7 +104,7 @@
-    "tabbable": "5.3.x",
+    "tabbable": "~6.0.0",

Why not ^ and not ~ this would be the first tilde constraint in this file. That feels wrong.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs work » Reviewed & tested by the community

#4: See the original version constraint; it was 5.3.x which is not same as ^6.0.0 which would equal >=6.0.0 <7.0.0. This could be changed to 6.0.x but IMO we should switch all of these eventually to use the tilde operator so that we can enforce minimum patch versions.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Needs work

I don't think we should be using tildes or 6.0.x - we should allow users to test and try out new minors without additional effort from us. All constraints in PHP land are ^. I don't think we should get in the way of users using 6.1.0 when it exists.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented

This was intentionally changed in #3266912: Review version constraints for production yarn dependencies to improve the maintenance DX.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented

The version constraints are dev dependencies. If users are comfortable modifying core shipped assets, they should be comfortable modifying the package.json too 🤷‍♂️

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Needs work » Reviewed & tested by the community

Oh okay... seems wrong to me but I'm not sure it matters in the long run.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 22f7fd0124 to 10.1.x and af3642897b to 10.0.x. Thanks!

  • alexpott committed 22f7fd0 on 10.1.x 
    Issue #3306167 by lauriii: Update to tabbable v6.0.0

  • alexpott committed af36428 on 10.0.x 
    Issue #3306167 by lauriii: Update to tabbable v6.0.0

(cherry picked...

Status: Fixed » Closed (fixed)

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