Problem/Motivation

This module will continue to be useful for Drupal 10. For example, to support sites using Bootstrap 3 (#3277732: Drupal 10 compatibility).

Currently, this module's jquery_ui.core library extends core's jquery.ui library, but that will be removed in #3277744: Actually remove deprecated jquery_ui libraries from core, so we'll need to decide on a different approach for this.

Steps to reproduce

Proposed resolution

As of nov 10 2022 the proposed solution implemented is the MR is option D: committing built files to the d.o repository, and only committing built files in jquery_ui, not in the various sub-modules such as jquery_ui_tabs, jquery_ui_slide, etc.

1. End-user experience

jQuery ui is used by about 27% of D8+ sites today (112188 sites) according to d.o project usage data. Because we point people to it in the deprecation message that number is going to increase in the short term and until jquery ui is removed from the various custom and contrib module/themes using it (so, not soon).

  1. This means that this module should be a "composer require and forget", after the require the module is functional: no nodejs required, no running a build step, people can go in the project browser, install it and it just works. Regardless of how we handle assets.
  2. The user need to be able to update the jquery UI version independently of updating the Drupal module. A new jQuery UI release should not mean the module needs a new release. It suggest having a way to monitor the latest version in the module and comparing to the installed version.

Do we agree on the target user experience? Before we dig in the technical discussion we should align on this first.

2. Module management/maintenance on d.o

It seems like we agree that having individual jquery ui component in it's own module is not ideal.

Regardless of how we handle the assets, would deprecating all the jquery_ui_* modules and regrouping everything in jquery_ui would be something acceptable? That would also simplify asset handling. We would lose the component usage data but it's not that critical.

  1. jquery ui module should replace the core assets to keep a single consistent version loaded (since we can update the version at any point per 1.2. above)

3. Library definition/component separation

Context for later: we have always split the jquery ui library by component so that one jquery ui component = one drupal library definition. That is because jquery UI packaged in a single file is big 69kb gzip (255kb unziped), for reference what core uses:

  • dialog only: 37kb gzip (86kb unziped)
  • autocomplete only: 20kb gzip (35kb unziped)
  • dialog + autocomplete: 43.7kb gzip (105kb unziped)

Using the existing jquery-ui.min.js as a whole hosted in CDNs adds twice or three time as much JS as the current component-based libraries.

Using the unminified files from the cdn such as widget.js has an unnecessary impact on file size. The source version of the widget.js file is 20kb unziped and the minified version is 8.5kb unziped.

  1. We need to keep each component separated => a step between the official jquery-ui npm package and the end-user is necessary.

4. Extra step/Asset handling

Assumptions:

  • Respects target user experience in 1.
  • A single jquery_ui module, see 2.
  • We keep a lib definition for each jquery ui component, see 3.

If we don't have the same assumptions we can stop here and align before going technical. I'm not trying to address the security concerns because I'm not 100% familiar with them so I don't want to make wrong assumptions, the implications should be pretty clear in each case though.

A. The bad solution

One possibility we didn't discuss is to opt-out of security coverage for the jquery ui module altogether.

B. Serve assets from a CDN

In this scenario *someone* would create a npm package that has all the built assets like we need them, and publish that on NPM (not under @drupal namespace of course). And use one of the auto CDN to point to the files in the module library definition. A setting could be available to serve the files from the CDN (default) or from the libraries/ directory if the user install them manually somehow.

This introduce a couple of problems:

  1. there is now a js package to maintain/trust (outside of core js packages)
  2. If we host this pacakge as a general project on d.o, then jquery ui code still ends up on d.o
  3. We trust a CDN with a third of all the Drupal sites, and hope nothing changes unexpectedly

I do not think this is a good solution. It simply moves the problem somewhere else (or not even in case of a general project) and adds a couple of new moving parts.

C. Composer magic aka. PHP built step

I think it's possible to do the whole build pipeline in PHP: essentially we would download the zip file for jquery-ui from npm, unzip it, run some php minifier over the files, copy them in a libraries/ folder somewhere so they'll be picked up automatically.

This fit with target UX, but has some problems:

  1. adds some significant processing for 100+k sites
  2. PHP-based minification is less good than what we have today

C'. Composer magic aka. PHP copy/paste

An alternative version of C. would be to get the Drupal-custom package created in scenario B. and unzip it in the libraries folder. Beside the CDN, same problems as scenario B.

  1. there is now a js package to maintain/trust (outside of core js packages)
  2. If we host this pacakge as a general project on d.o, then jquery ui code still ends up on d.o

D. Host built jquery ui assets on d.o

Adapt a version of the core vendor-update script to automatically copy/build the jquery ui files from the official npm package, like we do in core. Commit the built/minified files to the module.

No disruption for end users, no extra php code involved, additional maintenance is the vendor-update script run by the jquery_ui module maintainer.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#47 jquery_ui-3277748-47.patch1.27 MBnod_

Issue fork jquery_ui-3277748

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

effulgentsia created an issue. See original summary.

effulgentsia’s picture

Issue summary: View changes

balintpekker made their first commit to this issue’s fork.

lauriii’s picture

The library definitions are being removed in Drupal 10, but the assets will be still in Drupal core, because Drupal core uses jQuery UI internally. We could potentially just update the library definitions to reference the files from Drupal core, since it would be extremely unlikely that Drupal core would remove or move the assets before Drupal 11.

We should probably also do the same step for all of the jQuery UI submodules. Instead of having the assets in their repository, the submodules could depend on the files in core. This would make the maintenance easier and make Drupal core solely responsible for keeping the assets up to date. This would require adding the assets back to Drupal core that have been already removed but we could try to get a buy in on that from the Drupal core maintainers given that it should not add too much extra work for the maintainers.

catch’s picture

If we added assets for removed libraries back to core, this would mean core would be on the hook for any security updates to those libraries, which is likely to be the only reason they get an update (unless there's a jQuery 4 compatibility release from jQuery UI). Also if jQuery UI goes out of support during Drupal 10's lifetime, we'd have unsupported dependencies shipped with core. The whole idea of removing the libraries was to remove the responsibility from core for shipping them, and it's entirely possible we'd remove the assets in a minor release if we completely remove the dependencies.

I can see referencing the core assets that are there while they're there, because this would help with avoiding loading two different versions of the same file, as long as it's understood that this might require a change after a core minor release. But we shouldn't add assets back for this purpose, it'd be a side-effect of the ones we haven't completely removed yet.

nod_’s picture

Oh right if we added back what we removed (position, tabbable) we'd still have to deal with thing like slider, spinner, etc.

lauriii’s picture

Discussed with @nod_ and based on #4 we could rely on the core assets when they are available and the rest of the assets we would include in this module. However, I'm wondering if that would be prone to issues if the files are from different version of jQuery UI, e.g. core has jQuery UI 1.14.0 and jquery_ui has 1.13.2 (this module getting ahead we could prevent by managing version constraint in this module).

I guess an alternative approach would be for this module to override any references to jQuery UI in core to load jQuery UI from this module instead of from core. This way we would have consistent version of jQuery UI for all assets covered by this module.

A concern that was raised earlier by @xjm was that including files in these modules is against the Drupal.org policy on 3rd party assets. I discussed with @nod_ to try to come up with solution that would allow us to remove the assets from these modules. We thought it would be best if we could keep the assets in the modules because we comply with all of the steps from https://www.drupal.org/docs/develop/git/setting-up-git-for-drupal/drupal... except step 3.

When we start working on this, we should ensure that we have tooling to keep this module updated, and that we have documented the steps required for updating the assets. As we do that, we probably should create a version of the core vendor-update that is specific to jQuery UI so that the assets for this module can be built in a straight forward way. I'm also hoping that we could repurpose that for the other jQuery UI modules.

jungle’s picture

FYI, the asset source of jquery_ui_datepicker is not shipped in Drupal Core, checked against Drupal core 9.5.x, and this fact should be considered probably.

https://www.drupal.org/project/jquery_ui_datepicker

xjm’s picture

Regarding concerns about different versions of jQuery UI between core and the contrib module, I think the jQuery UI module needs to either reference the core assets, or be maintained parallel to core so that it is always updated when core is updated. That would need to be part of the proposal.

I don't think maintaining the assets in the contrib modules is acceptable. It's definitely against security team policy, and it prevents site owners from updating to secure versions when there is a security release. Right now core maintainers are more or less on the hook for maintaining like 30 contrib projects (but aren't actually maintaining them), most of which currently have outdated versions of their relevant libraries. This also forces the Security Team to issue security advisories for these 30 modules whenever the upstream library has a security release. It's a lot of work and overhead and has already had concerns raised from members of the Security Team with the last security release for Datepicker.

A better option would be to provide a script with the modules to build and install the specific library, with project setup instructions for using the command. Maybe the script could be provided with the contrib jQuery UI module, since each specific library should already have a dependency on that. The instructions would include how to run the script from the jQuery UI module to build the asset into the correct location.

nod_’s picture

Couple of topics here, i'll keep the asset handling as the last point since it depends on some of the answers to other topics.

1. End-user experience

jQuery ui is used by about 27% of D8+ sites today (112188 sites) according to d.o project usage data. Because we point people to it in the deprecation message that number is going to increase in the short term and until jquery ui is removed from the various custom and contrib module/themes using it (so, not soon).

This means that this module should be a "composer require and forget", after the require the module is functional: no nodejs required, no running a build step, people can go in the project browser, install it and it just works. Regardless of how we handle assets.

Do we agree on the target user experience? Before we dig in the technical discussion we should align on this first.

2. Module management/maintenance on d.o

It seems like we agree that having individual jquery ui component in it's own module is not ideal.

Regardless of how we handle the assets, would deprecating all the jquery_ui_* modules and regrouping everything in jquery_ui would be something acceptable? That would also simplify asset handling. We would lose the component usage data but it's not that critical.

As Lauriii proposed, I agree with the fact that the jquery ui module should replace the core assets to keep a single consistent version loaded. And yes, jquery ui will need to follow core and release at the same time as we update something jquery UI in core.

3. Library definition/component separation

Context for later: we have always split the jquery ui library by component so that one jquery ui component = one drupal library definition. That is because jquery UI packaged in a single file is big 69kb gzip (255kb unziped), for reference what core uses:

  • dialog only: 37kb gzip (86kb unziped)
  • autocomplete only: 20kb gzip (35kb unziped)
  • dialog + autocomplete: 43.7kb gzip (105kb unziped)

Which means that relying on the existing jquery-ui.min.js hosted in CDNs is a no-go.

We need to keep each component separated => a step between the official jquery-ui npm package and the end-user is necessary.

4. Extra step/Asset handling

Assumptions:

  • Respects target user experience in 1.
  • A single jquery_ui module, see 2.
  • We keep a lib definition for each jquery ui component, see 3.

If we don't have the same assumptions we can stop here and align before going technical. I'm not trying to address the security concerns because I'm not 100% familiar with them so I don't want to make wrong assumptions, the implications should be pretty clear in each case though.

A. The bad solution

One possibility we didn't discuss is to opt-out of security coverage for the jquery ui module altogether.

B. Serve assets from a CDN

In this scenario *someone* would create a npm package that has all the built assets like we need them, and publish that on NPM (not under @drupal namespace of course). And use one of the auto CDN to point to the files in the module library definition. A setting could be available to serve the files from the CDN (default) or from the libraries/ directory if the user install them manually somehow.

This introduce a couple of problems:

  1. there is now a js package to maintain/trust (outside of core js packages)
  2. If we host this pacakge as a general project on d.o, then jquery ui code still ends up on d.o
  3. We trust a CDN with a third of all the Drupal sites, and hope nothing changes unexpectedly

I do not think this is a good solution. It simply moves the problem somewhere else (or not even in case of a general project) and adds a couple of new moving parts.

C. Composer magic aka. PHP built step

I think it's possible to do the whole build pipeline in PHP: essentially we would download the zip file for jquery-ui from npm, unzip it, run some php minifier over the files, copy them in a libraries/ folder somewhere so they'll be picked up automatically.

This fit with target UX, but has some problems:

  1. adds some significant processing for 100+k sites, pretty unnecessary work too
  2. PHP-based minification is less good than what we have today

C'. Composer magic aka. PHP copy/paste

An alternative version of C. would be to get the Drupal-custom package created in scenario B. and unzip it in the libraries folder. Beside the CDN, same problems as scenario B.

  1. there is now a js package to maintain/trust (outside of core js packages)
  2. If we host this pacakge as a general project on d.o, then jquery ui code still ends up on d.o

D. Host built jquery ui assets on d.o

My preferred solution: adapt a version of the core vendor-update script to automatically copy/build the jquery ui files from the official npm package, like we do in core. Commit the built/minified files to the module.

No disruption for end users, no extra php code involved, additional maintenance is the vendor-update script run by the jquery_ui module maintainer.

catch’s picture

3. Library definition/component separation
Context for later: we have always split the jquery ui library by component so that one jquery ui component = one drupal library definition. That is because jquery UI packaged in a single file is big 69kb gzip (255kb unziped), for reference what core uses:

dialog only: 37kb gzip (86kb unziped)
autocomplete only: 20kb gzip (35kb unziped)
dialog + autocomplete: 43.7kb gzip (105kb unziped)
Which means that relying on the existing jquery-ui.min.js hosted in CDNs is a no-go.

Is it really a no-go? It would be a no-go for core, but this is a fallback until contrib has refactored away from jQuery UI, so if it simplifies everything, then using the bundled version from jQuery seems like it should be an option? Especially if there's also an individual-component option via https://www.drupal.org/project/libraries or similar.

nod_’s picture

Compared to an individual component solution we'd add 2x or 3x the amount of JS for the same thing (and the CSS too). That's pretty significant, that impacts page load, lighthouse score etc. Possible that people would see their web vitals score go down and create unnecessary noise when it reaches management (which is already what usually happens when lighthouse changes their scoring algorithm).

For comparison on an edit page with autocomplete:

  • currently: lighthouse score performance (mobile): 88, loaded JS: 147kb gzip (443kb unziped)
  • single file: lighthouse score performance (mobile): 87, loaded JS: 195kb gzip (563kb unziped)

So I don't think it's a good solution, at least I'd want to consider another solution first.

xjm’s picture

Issue tags: +Needs security review

The approach of committing the assets to the one contrib project (instead of 30) might be an acceptable compromise, but it'd need Security Team approval since it is still against established policy. Tagging for that.

xjm’s picture

Dealing with this again today for #3300040: Update jQuery UI to the latest versions, and knowing we have to also update contrib on top of that, I am unenthusiastic ATM about committing the assets to even one contrib repo.

catch’s picture

Compared to an individual component solution we'd add 2x or 3x the amount of JS for the same thing (and the CSS too). That's pretty significant, that impacts page load, lighthouse score etc. Possible that people would see their web vitals score go down and create unnecessary noise when it reaches management (which is already what usually happens when lighthouse changes their scoring algorithm).

Maybe that would provide some incentive for them to contribute to decoupling core and contrib further from jQuery UI.

nod_’s picture

@xjm, I would say it's better that a few maintainer run into a process snafu compared to 100k+ sites individually

@catch, I'm not sure it put the incentive in the right place, will that impact people who can actually solve that problem? I'm assuming people install the module because we told them so somewhere, not because they choose to. I guess if enough people complain something will be forced to move. But is that worth the user frustration?

DamienMcKenna’s picture

Why can't we just rely on composer dependencies on the jQuery UI module (with the assumption of deprecating the submodules)? Is it to specifically support folks who are not using Composer to manage their codebases?

nod_’s picture

If we had minification of js in core, we could get the unminified files from the CDN and serve the minified ones, or like DamienMcKenna mentionned, get composer to DL them on install somehow.

Serving the unminified version is not a good idea, the files have lots of comments and the size is easily x2 for the files, that would make it in line with serving the whole thing all the time at that point.

xjm’s picture

@xjm, I would say it's better that a few maintainer run into a process snafu compared to 100k+ sites individually

This would be true if core maintenance of this was sustainable. It's not. The release managers and the sec team do not have the resources to reissue SAs on behalf of third-party dependencies that we copy to our codebase. As I said in the core-recommended issue, (A) noisy reissued SAs for every upstream vuln, or (B) on-time, relevant security, minor, and major releases for Drupal core. Pick one. We don't have the resources for both. I'm not aware of any other project that does this, reissuing a dependency's SAs as the project's own, when there's not even a vulnerability in Drupal to fix.

In general, it's better to put the ability to update third-party dependencies in the site owners' hands. We deal with so much frustration from people with every upstream security release that they have to wait for us to update the dependency because we forced them to use a specific, older version. Contrib maintaining assets would have the same problem.

Therefore, after thinking about it for a few days, I think I've come around to it needing to be an installation script again. Maybe not with Node if we think that's too steep a hill -- maybe a Composer-integrated step with less-perfect PHP minification of assets etc. We can explore ideas along those lines. We can wait until more of the team have given feedback, but I actually feel very strongly about this after actively having to evaluate one of these releases this week. (Earlier in the week when we first discussed it I was more ambivalent.)

To me, it's like the work we put into building sensible JS asset management tooling for core. Before, updating JS deps was a nightmare. Now, it's clearly documented and easy to learn even for PHP developers. The development time to get to that point was signficant, but it saves so much pain every time we update something. I think we can provide site owners something like that and it will be worth it, so long as we're not in any way locking them to one specific version of the dependency.

nod_’s picture

Issue summary: View changes

Updated the IS, added a section to the end user experience part and added a few things about the cdn/unminified files.

The user need to be able to update the jquery UI version independently of updating the Drupal module. A new jQuery UI release should not mean the module needs a new release. It suggest having a way to monitor the latest version in the module and comparing to the installed version.

Given the new user requirements I'm leaning towards solution C

nod_’s picture

Issue summary: View changes
Supreetam09’s picture

jQuery-UI from these contrib modules have started throwing vulnerabilities (checked with retire.js extension) since these contrib modules are using 1.12.1 version whereas Drupal core 9.5.x now have 1.13.2. The latest version 1.13.2 does not have any vulnerabilities. It will soon become critical IMO.

Supreetam09’s picture

Will it be possible to release a new version for all jQuery UI modules for latest 1.13.2 version. At least until we come to the solution for D10?
OR maybe to reduce the hassle of asset handling combine all them to a single module with 1.13.2 jQuery UI version for now and deprecate all other modules as a temporary solution?

lauriii’s picture

Berdir made their first commit to this issue’s fork.

Berdir’s picture

Status: Active » Needs review

This is IMHO shaping up to be one of the major D10 contrib blockers. The issue summary outlines the options we have quite nicely and while I understand that essentially replicating what core does right now is bad for maintainers (option D), it is what we can do today, without any major tool changes, which I expect will take time.

And then we can unblock lots of contrib projects and work on a better version that can replace this.

I didn't know that this can be automated with that core script, so that means that extra work is annoying, but it's literally running one script, committing and tagging that and then a new version is out. There are have been exactly 3 releases so far in a year. I'd also be fine with not having security coverage from the security team for this project, clearly communicating that you are basically on your own. If some big companies have a problem with that, they're welcome to throw money at the security team to have more paid time there :)

So I sat down and started the implementation of that, to see if it would work and it seems to work fine, at least as far as building the JS scripts go.

The update vendor script could certainly be improved, I really just deleted all the lines that weren't needed anymore. I had to fix the libraries.yml detection, because core incorrectly checks for library position not being 0, assuming that the first line would not be a vendor library definition. Needed to add a package.json with the dependencies that I think are needed, run the script and copy over the library definitions 1:1 from core.

I only did it for those that were already in the module, but all assets are included and it should be easy to add the others as well.

I did rename the base library name as core seems weird and also doesn't work with the script (but that could be hardcoded to just look for and replace the jquery ui version), either way this should probably be a major new version as it will also replace tons of other projects per the suggested approach in the issue summary.

Pushed that to the open but so far empty merge request.

Wim Leers’s picture

Priority: Normal » Critical
Issue tags: +Contributed project blocker

This is IMHO shaping up to be one of the major D10 contrib blockers.

+1

lauriii’s picture

I'd personally be fine with the approach D) until #2873160: Implement core management of 3rd-party FE libraries has been resolved. It looks like @nod_ who's also maintainer of this module seems fine with it too based on #10. I understand that coordinating releases with jQuery UI requires some work. However, we will have to do that regardless of what we do here until Drupal 9 is EOL.

As I already mentioned in #7, based on my personal interpretation of the policy as the maintainer of the module, I believe that this module should be eligible for the exception to host the 3rd party assets in Drupal.org. Regardless of that, based on #14 and #19 it may be that this approach is not acceptable or preferable to the security team. I'm hesitant to go with an approach that would not take into that feedback, especially given that this module has a special relationship with core given that the core is recommending sites to migrate to using this module.

As a next step, I will try to reach out to the security team to get more feedback from them.

greggles’s picture

I've tried to read the issue and comments, but will admit to skimming so apologies if I missed some key points.

I can see some pros and cons to option D from the security team perspective. Our goal is generally to keep all Drupal sites secure and option D seems like the one that would best help sit maintainers to keep their sites secure.

I also agree with some of the feedback in comments so far that the security team does not have the time to help with an SA for every jquery UI upstream that has a problem.

The security team's effort is dramatically reduced if there is a maintainer who knows the process and can make a release 3 wednesdays a month with as little as 1 day notice. I don't necessarily think that would be a good use of the maintainers time vs. the security team, but if there were a maintainer who wants to do it and especially a reliable sponsor who wanted to make that happen with a person who knows the process very well then I would be more excited to go down the path of hosting the code on drupal.org until #2873160: Implement core management of 3rd-party FE libraries is fixed.

None of this is consensus of the security team nor an official position, just sharing my individual thoughts for what it's worth.

lauriii’s picture

It doesn't seem like there would be significant burden added to anyone from having the assets inside the module as long as core is responsible for handling the updates. So I'm wondering if we can get consensus that we could keep the assets inside this module until the EOL of Drupal 9?

If we can't commit to maintaining those assets beyond that, we could potentially EOL the current major of this module when Drupal 9 is EOL and ship a new major without the assets. With the new major, users of the module would be responsible for getting the library and keeping it up-to-date. If we can get #2873160: Implement core management of 3rd-party FE libraries done by then, we could utilize it.

Berdir’s picture

> It doesn't seem like there would be significant burden added to anyone from having the assets inside the module as long as core is responsible for handling the updates. So I'm wondering if we can get consensus that we could keep the assets inside this module until the EOL of Drupal 9?

I don't understand this, those two sentences seem to contradict each other? I guess it seems somewhat likely that the jquery.ui libraries won't be removed within a year, there's quite some work left and it doesn't really simplify anything to only remove some, exactly because we have it automated with that script. This issue really is about what happens afterwards.

My proposal in the merge request does copy the assets in the module so they are maintained here. Because this comment from @greggles is a very good point:

> I can see some pros and cons to option D from the security team perspective. Our goal is generally to keep all Drupal sites secure and option D seems like the one that would best help sit maintainers to keep their sites secure.

We all know that nobody every updates their manually downloaded libraries in D7 projects (which ckeditor version are you using...). Having them shipped in this module means nobody needs to do anything special, it just works if they update this module. As mentioned, jquery_ui had exactly 3 releases in a whole year, 2 of those were security updates. Yes, it could happen anytime (I do hope that we can get in touch with them and get early warnings on that though), but not 3 times per month, there's just a slight chance that it could happen and then we need to act until the next security release window (or outside of it, as it's really already public at that point).

Note on #2873160: Implement core management of 3rd-party FE libraries, I think many projects are already using composer for node dependencies, either with asset-packagist or some other solution. We moved away from that and just specify hardcoded repositories for each dependency, but that's fairly tedious to update. But we've been discussing that for 5 years and I think a short-term resolution is unlikely, and anything that requires actual local node runtimes seems unlikely to be adopted by users that already complain about composer. It also doesn't help anyone who does not use composer.

One more important point by nod earlier is that our script splits the whole library into separate optimized packages, something that _is not possible_ through any of the other available npm or CDN sources AFAIK.

nod_’s picture

Assigned: Unassigned » nod_

working on improving the MR that implements option D. Short term, I don't have a better solution to unblock things.

the requirement "The user need to be able to update the jquery UI version independently of updating the Drupal module." put in the context of "We all know that nobody every updates their manually downloaded libraries in D7 projects (which ckeditor version are you using...)." means that option D is the least bad solution for now.

nod_’s picture

Assigned: nod_ » Unassigned

the script has been improved to build the libraries.yml file totally automatically (including dependencies), that way there is nothing to do manually if/when jquery ui changes their libraries or dependencies.

The idea is that all the JS code lives in the jquery_ui module and sub modules such as jquery_ui_tooltip will not have any js code in them, and they will declare a library as an alias to an existing jquery_ui library :

tooltip:
  dependencies:
    - jquery_ui/widgets.tooltip

That way we can leave all the sub modules alone for ever and only need to update the jquery_ui module for everything to be up to date.

The code needs a bit of clean-up and it needs to be commented a bit too, should be working though.

nod_’s picture

Ready for review

Berdir’s picture

> That way we can leave all the sub modules alone for ever and only need to update the jquery_ui module for everything to be up to date.

In the issue summary is this:

"Regardless of how we handle the assets, would deprecating all the jquery_ui_* modules and regrouping everything in jquery_ui would be something acceptable? That would also simplify asset handling. We would lose the component usage data but it's not that critical. "

That would mean there won't be any submodules anymore and all modules just need to depend on jquery_ui. I think that would be great and would simplify things a lot. https://www.drupal.org/project/better_exposed_filters for example depends on 3 projects and is trying to get rid of them, so each of them has to do a complicated dance with module dependencies that conflict between each other.

effulgentsia’s picture

I can't speak for other security team members or other core committers, but for what it's worth, I think that option D (include the jquery_ui files in the module, which is what this issue's current MR does) is a good option until Drupal 9's EOL (Nov. 2023).

As #29 says, until then we already will have to issue SA's for Drupal 9 core when jQuery UI releases security fixes. Assuming we have a script to generate the needed commit to D9 core, and if essentially the same script could generate the commit to this module, then releasing an SA for this module at the same time as we release it for D9 core, while adding some additional work on top of the D9 core SA alone, seems like an addition that's on the lighter side of all the things that burden the security team.

Something that could make the process even smoother would be if Drupal core's release managers are given maintainer access to this module, in order to be able to commit and tag a release to this at the same time they do it for core, in the event that this module's other maintainers aren't available that day.

Beyond Nov. 2023, I don't know what the right solution is. I guess we'll need to see where we're at on #2873160: Implement core management of 3rd-party FE libraries by then.

lauriii’s picture

I guess It's worth noting that Drupal 10 also ships with a subset of jQuery UI components, meaning that there will be ongoing overlap with core processes and this module. Because of this, it doesn't seem catastrophic to me even if we have to continue with the current process beyond November 2023.

I reviewed the MR and didn't spot anything ground breaking – overall all looks good even if we shipped it as it is. I left couple comments just to make sure that everything is in as clear state as possible for anyone to jump in on this later.

That way we can leave all the sub modules alone for ever and only need to update the jquery_ui module for everything to be up to date.

Is the plan to continue supporting and recommending the submodules? If we make the libraries directly available from this module, doesn't that render the submodules useless since the data would no longer be reliable? Should we mark all of them deprecated as we update them to use libraries from this module? That would also mean we'd have to update some of the upgrade related documentation.

Something that could make the process even smoother would be if Drupal core's release managers are given maintainer access to this module

I'm personally happy to give commit + release access for this module to any of the core release managers.

nod_’s picture

Thanks for the review, updated code.

effulgentsia’s picture

Would it help us in the long run if we made the libraries for the individual widgets (e.g., widgets.datepicker) "internal", where the only "allowed" user of them was the corresponding component module (e.g., jquery_ui_datepicker) which could list the internal.widgets.datepicker library from this module as its dependency, but if anything else lists internal.widgets.datepicker as a dependency, then we either disallow that entirely or emit a deprecation notice?

That way, we still keep all of the assets centralized in this module, but Drupal modules need to go via the various jquery_ui_* modules, and that way, we collect usage data for them, and if in the future there's other benefits to the separate modules, then we can get those benefits without requiring Drupal users to change anything in their code. If all those modules need to do for now is to depend on the corresponding internal library from this module, then seems like we can make a 2.0.0 release for all of them and they won't need another release for a while; we'll just need to populate the "version" and "license.url" keys of their library definitions dynamically based on those values in the corresponding internal library definition.

Wim Leers’s picture

I really like #40 🤓

nod_’s picture

prefixed by internal for the auto discovered libraries.

smustgrave’s picture

Also like the idea of #40. We are prepping a site for D10 and a number of modules are blocked by a jquery dependency needing updating.

nod_’s picture

Assigned: Unassigned » nod_
Status: Needs review » Needs work

Trying to make a version that doesn't need the intermediate "internal" libraries.

nod_’s picture

Trying things out with #3319058: Drupal 10 version asset management (datepicker) and #3319057: Drupal 10 version asset management (slider) (taking the 2 most used jquery ui modules, with datepicker having a few things specials to take care of).

nod_’s picture

Assigned: nod_ » Unassigned
Status: Needs work » Needs review

So with this merge, request and the patches to the two other issues the better_exposed_filter module works as intented. that's the module driving usage for the datepicker and slider modules.

nod_’s picture

FileSize
1.27 MB

uploading a patch since gitlab is acting up.

The magic happens in the jquery_ui_library_info_alter function.

phenaproxima’s picture

  1. +++ b/jquery_ui.module
    @@ -21,3 +22,40 @@ function jquery_ui_help($route_name, RouteMatchInterface $route_match) {
    +function jquery_ui_library_info_alter(array &$libraries, $module) {
    

    Can we type hint $module as well, and add a void return type hint on this function?

  2. +++ b/jquery_ui.module
    @@ -21,3 +22,40 @@ function jquery_ui_help($route_name, RouteMatchInterface $route_match) {
    +    $data['libraries'] = Json::decode(file_get_contents(__DIR__ . '/jquery_ui.libraries.data.json'));
    

    I'd recommend calling json_decode() directly so we can take advantage of JSON_THROW_ON_ERROR.

  3. +++ b/jquery_ui.module
    @@ -21,3 +22,40 @@ function jquery_ui_help($route_name, RouteMatchInterface $route_match) {
    +    if (in_array($widget, array_keys($data['libraries']))) {
    

    We could just use array_key_exists() here.

  4. +++ b/jquery_ui.module
    @@ -21,3 +22,40 @@ function jquery_ui_help($route_name, RouteMatchInterface $route_match) {
    +function _update_asset_path($path, $module_path) {
    +  return '/' . $module_path . '/' . $path;
    +}
    

    Not sure if this is an actual issue, but will this work if Drupal is installed in a subdirectory?

nod_’s picture

1. done
2. changed
3. d'oh!
4. Shoudln't be a problem. This is an path on the filesystem of the server.

smustgrave’s picture

What's the best way to test this?

I have the jquery_ui_draggable module installed.
Applied the changes to the jquery_ui module
I don't see any library from jquery_ui_draggable being loaded (which I think is expected based on the hook)
But the draggable feature still works.

If that's the desired behavior +1 RTBC

nod_’s picture

Yep that's pretty much it.

Techincally it's best to get new versions of the jquery_ui_* modules, but the alter hook makes it so that it works as is just by updating jquery_ui module.

So yes, the desired behavior is that everything is loaded from jquery_ui module and that the features still work.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

In that case ill mark it

phenaproxima’s picture

Status: Reviewed & tested by the community » Needs work

Found a few things in the code review.

nod_’s picture

Status: Needs work » Needs review

added the effects libraries.

Now it seems the weight for the version-min.js file is messed up somewhere and it causes some problems. Can someone else try it out see if it's just me?

nod_’s picture

Status: Needs review » Needs work

All right, tried very hard to make it work without exposing unnecessary libraries but it won't work. Having many times the same file added by different libraries work in theory but in practice it messes up the weight calculation and creates ordering issues.

So I'll go back to having everything declared in dependencies. Only thing I could do is declare the "extra" libraries we don't want people to see in the info/alter hook so that they don't show up in the libraries.yml file of the jquery_ui module. Not foolproof but you'll have to work to make the wrong thing.

nod_’s picture

Status: Needs work » Needs review

All right I think that's the last version.

Code is simplified and there are no load order issues.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Testing on my D10 site and appears to be working. Will there be coordination with the subcomponents to upgrade? They're big blockers for a few modules.

nod_’s picture

Issue summary: View changes

  • nod_ committed 4027599 on 8.x-1.x authored by Berdir
    Issue #3277748 by nod_, balintpekker: Drupal 10 compatibility
    
nod_’s picture

Status: Reviewed & tested by the community » Fixed

All right, it's in.

Also the commit message is all wrong, sorry about that. updating credits here at least.

Status: Fixed » Closed (fixed)

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

joseph.olstad’s picture

This jquery_ui exit from core change has affected negatively the beautiful localisation approach we previously had with jquery_ui_datepicker version 1.x. I'm assuming the 2.x release was forced in due to the D10 core changes. 2.x relies on the latest release jquery_ui contrib module which supplies a broken implementation that only works in English.
I just wrote a patch to use a language other than English but then English is lost. I've spent nearly 12 hours on this so far for a broken half solution.

There's more to it than what appears at first.
#3339013: implement datepicker localisation in jquery_ui 2.x, compare with jquery_ui_datepicker 1.x that has localisation

It's quite complicated, very unfortunate how jquery_ui was jettisoned at high velocity out of core seems to somehow have resulted in a broken contrib solution that broke a beautifully working approach to localisation.

We had Drupal t strings doing months and weeks and days now we have a big mess.