Refactor drupal_get_js() to use render layer similar to drupal_get_css()

Wicked cool.

It doesn't really present anything. A script tag modifies behavior but I do like the idea of giving themers control because I think themes should have the final say in how a site works. I brought this up for drupal_get_css a long while back but I was laughed at. ;) It's sort of the same idea.

mfer, I would +1 in allowing the theme to override any drupal_get_* that adds tags to the html source. I'm not so sure it would be best to use the theme() function. That has traditionally been about presentable markup.

I am on the fence about this one.

I would like to see a use case where a themer would want different output. Since script tags are not presentation, truly, I am more concerned that themes would botch this for little to no gain. Let's put our thinking caps on and come up with a reason why this would be useful.

mfer: The latter sounds like an argument for an alter function rather than a theme function, or some settings as to how the function behaves rather than actually theming it. I'm not sure theming would undo an alter function, for example. In fact, all theming is going to get you is changing how the <script> tag is put together, isn't it?

Whereas, an alter function is what would let sf cache module take advantage of js min.

So what if we had:

1. hook_js_alter() to modify the whole $javascript array before presenting to the screen.... and......
2. theme_script() to theme how an individual script tag is presented on the page.

This means we keep drupal_get_js(), but just modify it to switch it to a theme('script') call, and add the call to drupal_alter beforehand?

I'm unsure of theme_script(). What would be changed there? How many variations of <script xx> do we need?

The only thing I can think of is IE conditional comments when applied to style sheets where the themer may want to allow aggregation. Themes currently have the only option of adding style tags manually with conditional comments preventing compression. Outside of that, I can't imagine another use case.

And hook_js_alter cannot be touched by themes? It'd be nice if it could.

Since the plan is to keep the functionality similar between drupal_get_js & drupal_get_css, as a theme dev I'm more concerned with the ordering of the style sheets and allowing the alteration of scripts by omitting them or adding my own in place of another. Having the ability to add a custom global reset style as the first style sheet currently requires custom code to accomplish.

Mfer created the pluggable aggregation issue here: #352951: Make JS & CSS Preprocessing Pluggable.

I imagine this should become "Allow alteration of the Javascript and CSS in the theme layer", or something like that?... And I believe it would have to wait until hook_css_alter is in: #356963: Allow CSS to be altered.... Does that make sense? I'm not too sure how I feel about that yet though. Seems like something that style.css and hook_js_alter should handle instead.... Needs more thoughts, I guess.

I still think that adding theme_script makes sense, since it generates HTML.

If we allowed themes to implement module hooks themselves, would we be violating one of the many rules of Drupal?.... Allow themes to implement hooks after all other module hooks have been invoked?

function garland_init() {
}
function garland_js_alter() {
}
// ........ etc.

Does this kill kittens and cute puppies at the same time?

Ran into this issue (sort of) again at http://drupal.org/node/511820 .

Having drupal_get_js() called so early definitely limits developers form doing things they'd like to do in a reasonably expected way.

What about adding a template_postprocess_HOOK() at the very end of the theme life-cycle (http://api.drupal.org/api/function/theme/6), where template_postprocess_page() could call drupal_get_js(), drupal_get_css(), etc ?

Then themers can do what they're already trying to do without having to learn a new hook.

I know the objection is that this will encourage bad practice, i.e., themers will do in the theme layer what should be done as a module, but to me, the above use case (where to put the comment form) is very much a presentation issue.

Why add hooks when we can accommodate existing assumptions without them? Better DX, IMO.

BTW, It might even be argued that calling drupal_get_form() in theme_preprocess_page() which causes the form to be rendered with everything but it's javascript, is a bug.

Finally, if this is too drastic, drupal_get_js, etc, could be moved to drupal_final_markup, although I think having if ($hook == 'page') in theme() is a bit hackish as it is. (Maybe I should actually read the surround code.) :-P

However, none of this necessarily renders obsolete the idea of a hook_js_alter -- but it does address the most common use cases, IMO.

Actually, a better solution might be in #591794: Allow themes to alter forms and page, as it exposes hook_js_alter() and hook_css_alter() to the theme layer.

Oh, hey, someone already invented template_process_html(). Way to go! Sorry for wasting bandwith. Ignore #17 above.

We have hook_js_alter(), and drupal_get_js() is called as late in the pipeline as possible (template_process_html()), so most of the issues here are solved, but there remains the very valid point that someone might want to override some of the stuff that's hard-coded into drupal_get_js().

In #228818: IE: Stylesheets ignored after 31 link/style tags, the idea of theme('stylesheets') was shot down on similar arguments as presented in the above comments (it's not strictly "presentation", so doesn't belong in the theme layer), but we finally settled on making it part of the render layer: see patch in #228818-377: IE: Stylesheets ignored after 31 link/style tags or just look at HEADs drupal_get_css() implementation.

Is there support among the people here to make the same change to drupal_get_js()? There's less of a use-case for this as in the other issue where people needed to ability to decide when to use a LINK tag vs. a STYLE tag. But I like consistency, and overridability, so I think applying the same pattern to drupal_get_js() makes sense even in the absence of a use-case. But this late in the D7 cycle, I'd like to see some support for it before rolling the patch. If someone else wants to roll the patch, even better!

What if instead of returning a string of all the script tags, it would return a renderable array? Then we wouldn't have to worry about sorting it because we could push it through to the #weight property. Might be unrelated to this, but it is an idea...

What if instead of returning a string of all the script tags, it would return a renderable array?

Too late to break APIs, including the return types of public functions like drupal_get_js(). But what I'm proposing is to do the same thing to drupal_get_js() as what is now in drupal_get_css(): move everything after the "// Filter out elements of the given scope." section to a #pre_render function, which would allow overridability and the use of #weight for sorting if that's desired.

Subscribe.

Drupal 8 territory at this point...

Sorry, didn't know of this issue.

Work on this refactoring has started over in #865536: drupal_add_js() is missing the 'browsers' option, where we actually have a concrete use-case for bringing drupal_get_js() in line with drupal_get_css().

Therefore, marking as duplicate.