So I originally wanted to try to figure out how support for external libraries/variants should ideally work, but in trying to do so, I ran against many little walls in the current API:

http://drupalcode.org/viewvc/drupal/contributions/modules/jquery/

The only/main code is in jquery.module and is mostly a quick hack only. However, I've added plenty of @todos in the process.

As most of those issues are revealing fundamental problems in the current API, it would be wise to focus on them first; i.e., before trying to add new features to the API.

It's possible that some @todo issues are covered by other issues/patches in the queue already. We'll have to figure out the order of @todos to tackle, and which of them should be tackled in a separate issue.

Comments

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Simply copying the todos from jquery.module here for reference. I'm not sure they all are related to Libraries API, but since it's not THAT many, that shouldn't be a problem.

  1. l. 40f, jquery_library_alter(): @todo Libraries API should transpose + default all 'files' definitions to always use the array structure, too.
  2. l. 63, jquery_libraries_info(): @todo Downloaded library is not an archive but the JS file itself, and contains the library version and variant; e.g., jquery-1.4.3.min.js. Not a single 'files' property can be defined, because the values are always different. Same applies to the 'file' argument of 'version arguments'.
  3. l. 67, jquery_libraries_info(): @todo Default version callback documentation, phpDoc, and example pattern is wrong. 'pattern' always needs to capture the entire version string in \1.
  4. l. 69, jquery_libraries_info(): @todo Local/remote variants are different kind of variants than the usual source/minified/etc variants. They are additional, not substitutes.
  5. l. 71, jquery_libraries_info(): @todo All variants are marked as installed by the version callback, even though only one variant exists.
  6. l. 121, jquery_libraries_get_version(): @todo Change respective 'files' definitions in $library accordingly. Ugh.

I hope to catch you on Skype this week, in which case the numbering here will makes things a bit easier...

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented

Thanks! I've slightly edited your comment + removed the @todos which aren't relevant for Libraries API

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
  1. For all 'files'-alike properties, we want to standardize an original definition of
      'js' => array('foo.js'),

    into

      'js' => array('foo.js' => array()),

    before any other function is invoked.

  2. Version- and variant-dependent library file names are a major problem. Perhaps special to jQuery related libraries, but we should come up with a sane way to supporting libraries with alternating file names, as that is clearly a job for the API.
  3. Wrong docs on 'pattern' should be clear, I think.
  4. Actually belongs into #864376: Loading of external libraries -- but yeah, externally hosted libraries aren't "variants", it's more or less only the file location that differs. There's still a source/minified/whatnot variant that can be potentially loaded.
  5. I hope/suspect that #864376 might fix this as well? :)
  6. Is actually the same as 2)
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

All right, thanks for the explanation.
1. Should be relatively easy, let's move that to #958162: Allow groups of callbacks to be applied to libraries.
2. Is an "oversight" (how could we... :) ) in our current API, I think that deserves its own issue: #964986: Support version- and variant-dependent filenames
3. Is pretty much trivial to fix, let's also move that to #958162: Allow groups of callbacks to be applied to libraries.
4. I had thought about the following before in #864376: Loading of external libraries, but had previously disregarded the thought for complexity: Maybe it is time to introduce a 'detection callback', which defaults to libraries_get_path(), and then we provide a useful default function for libraries that are also available externally to provide the alternate locations and define priorities or something. I also think this should be moved to #864376: Loading of external libraries.
5. Is definitely a bug, but I'm pretty sure that at least one test in #864376: Loading of external libraries would fail if that's still the case over there. We should definitely investigate that, though, but it makes sense to do that over there, since we're changing the "marking variants installed"-logic over there anyway.

Let's leave this open, though and mark it fixed, once jQuery.module is able to consume our API cleanly and hack-less.

ohnobinki’s picture

ohnobinki CreditAttribution: ohnobinki commented

sub

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Issue tags: +D7 stable release blocker
tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

Let's mark this fixed.
The issues this spawned off have been fixed, or are being debated.
One of those, that is not linked above, is #1224838: Fix the inconsistency between filenames as array keys and array values.

Let's try for better jquery.module support in 3.x :-)

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented
Status: Active » Fixed

fail

Status: Fixed » Closed (fixed)
Issue tags: -D7 stable release blocker

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

Issue tags: +D7 stable release blocker

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.