Now that JavaScript dependencies are defined in hook_library_info(), we can remove the weight as hook_library will add the JavaScript/CSS files in order according to their depending libraries.

CommentFileSizeAuthor
hook_library_info_weight.patch4.07 KBrobloach

Comments

nod_’s picture

I'm very curious to see if the tests pass :)

sun’s picture

I'm afraid, I think this makes the situation even worse than it is now. :(

Cross-linking original issue:
#1805452: [meta] Asset library declarations, registry, usage, DX, compatibility, documentation

robloach’s picture

If you want your library to be presented after jQuery, then have it depend on jQuery. The "weight" is a hack around not having dependency information available. Now that it's in there, the "weight" doesn't need to persist. JavaScript is added in the correct order, in accordence to the dependency heirarchy.

sun’s picture

No. The reason for #2 is:

Libraries are processed and added to the page in no particular order.

This means that the effective "weight" and position of a particular library would critically depend on when exactly it was added — which really means: at which exact point its dependencies were resolved and the library itself was added. This is multidimensional, including the time and space continuum.

There is a unlimited amount of occasions in which #attached libraries may be processed and added to the page. Essentially, every single call to drupal_render() throughout the page rendering process. Removing weights means to introduce a hard dependency on a single process invocation for resolving all dependencies for all libraries that may be added to a response in single, central spot (involving a static variable), which is not only able to vertically resolve dependencies but also horizontally. (EDIT: Essentially, building and processing a Directed Acyclic Graph on every single request.)

I've mentioned this major problem space in #1787758: drupal_get_library() / hook_library_info() is not cached already, but earned nothing but pushback/ignorance.

Furthermore, such a process is almost guaranteed to produce always differing aggregates. Essentially killing the very last sense of aggregation and grouping logic we have/had. We're on a good way to destroy frontend performance entirely for D8. If that's the goal, keep on moving.

robloach’s picture

Libraries are processed and added to the page in no particular order.

A library's dependencies are added to the page before the library itself is added. If your project's dependencies are broken, then yes, there will be conflicts. But, if you define the dependencies correctly, then the order will be retained as defined by the hierarchy. If you provide the use case in which you're running into problems, then a link would be great.

I've mentioned this major problem space in #1787758: drupal_get_library() / hook_library_info() is not cached already, but earned nothing but pushback/ignorance.

That's unfortute. We should probably re-open that one.

Furthermore, such a process is almost guaranteed to produce always differing aggregates. Essentially killing the very last sense of aggregation and grouping logic we have/had.

Agreed... This is why I'm much more in favour of moving to RequireJS, where loading the dependencies is handled for us.

Otherwise, what solution do you suggest for weights?

sun’s picture

To understand what I mean, please create a test case:

1) Register 3 libraries A, B, C.

2) B depends on A, C depends on B.

3) Consecutively add all libraries A,B,C and confirm that dependencies are resolved correctly and files appear in the expected order.

4) Repeat with all possible permutations and confirm that the resulting order is always identical:

ABC
BCA
CAB

BAC
ACB
CBA

[5) See it fail.]

nod_’s picture

Status: Needs review » Needs work
sun’s picture

As far as I understood @sdboyer's plan for using Assetic, that will actually implement and resolve the problem that I mentioned in #4:

Instead of resolving partial dependencies individually for each library that is attached, a full graph of all libraries is built and resolved only once prior to the page being rendered out.

When I last looked at that patch though (a few days ago), it was still far away from being commit-ready. Therefore, I wonder whether we can implement an interim step and

  1. make drupal_add_library()/drupal_process_attached() only pollute a variable somewhere, holding the libraries to attach.
  2. move the actual processing + dependency-resolution + attaching of libraries into the HTML page rendering process, and only execute it once for the full stack.
  3. → in turn, remove all weights from declared libraries (and make drupal_get_library() blatantly ignore/remove custom weight options when parsing the definitions)

Thoughts?

robloach’s picture

Status: Needs work » Closed (duplicate)

Yup!