This issue is used for tracking progress on cleaning up core javascript.

Problems

  • code is inconsistent,
  • code is not robust enough to be used with bad 3rd party code (extending prototypes),
  • there are flaws in the design of a number of components,
  • jQuery selectors are abused,
  • there is no proper place for javascript files in code folder structure,
  • javascript coding standard are incomplete #1778828: [policy, no patch] Update JS coding standards.

Requirements

Issues

javascript clean-up

jQuery clean-up

Done

I'm not sure it all applies, it's the kind of things I saw in contrib. and I suppose most of them can be found in core.

Benefits

Safer and faster execution of core JS (selectors simplification can go a long way here). It'll make refactoring go smoother too (one day, when file-level closures go away).

And having a strict code style literately fixes bugs #784670: autocomplete.js can iterate over functions in Array objects.

The first patch going this way just got commited to D8 \o/ #1400310: Use of .parents() is really .closest().

How does this fit with other initiatives?

We are in touch and share some issues with the mobile and html5 initiative.

How can I help?

If you are subscribing to this issue and want to help, note that there is a very long list of issues linked from the opening post that are dying out for reviews, re-rolls or initial patches so it should be easy enough to find somewhere to start.

If after looking at that list you don't know where to start, grab nod_, catch on IRC (#drupal-contribute) or just ask around, and we'll try to help you find something.

Individual files status

File clean-up Main issue JSHint
misc/ ajax.js #1533366: Simplify and optimize Drupal.ajax() instantiation and implementation #1684788: JSHint ajax.js
authorize.js #1574484: Selectors clean-up: authorize.js #1684790: JSHint authorize.js
announce.js #1999816: JSHint announce.js
autocomplete.js #675446: Use jQuery UI Autocomplete #1684792: JSHint autocomplete.js
batch.js #1574490: Clean-up: batch.js #1684794: JSHint batch.js
collapse.js #1685140: Refactor collapse.js
#1975426: Clean up collapse.js
#1684798: JSHint collapse.js
debounce.js [#]
dialog.js [#]
drupal.js #1089300: Clean up drupal.js
#1974580: Introduce domReady to remove jQuery dependency from drupal.js and clean it up.
#1684800: JSHint drupal.js
dropbutton/dropbutton.js #1799498: Improve dropbutton already ok
form.js #1685146: Refactor form.js #1684802: JSHint form.js
jquery.once.js [#]
machine-name.js #1686174: Refactor machine-name.js #1684804: JSHint machine-name.js
matchmedia.js #1835094: JSHint matchmedia.js
progress.js #1751044: Selectors clean-up: progress.js
#1810934: Adding custom ajax progress indicators more easily
#1684806: JSHint progress.js
states.js #1751070: Selectors clean-up: states.js #1684810: JSHint states.js
tabledrag.js #1524414: Rewrite tabledrag.js to use jQuery UI #1684812: JSHint tabledrag.js
tableheader.js #1574738: Rewrite tableheader.js
#1764912: Fix regressions and further improve tableheader.js
#1684814: JSHint tableheader.js
tableresponsive.js already ok
tableselect.js #1751308: Refactor tableselect.js #1684816: JSHint tableselect.js
timezone.js #1574696: Selectors clean-up: timezone.js #1684818: JSHint timezone.js
vertical-tabs.js #1751312: Rewrite vertical-tabs.js #1684820: JSHint vertical-tabs.js
modules/ block #1574512: Selectors clean-up: block #1684822: JSHint block
book #1574620: Selectors clean-up: book #1684826: JSHint book
ckeditor [#] #1999810: JSHint ckeditor
color #1751334: Selectors clean-up: color module #1684830: JSHint color
comment #1751340: Selectors clean-up: comment module #1684832: JSHint comment
contextual #1751344: Selectors clean-up: contextual module #1684834: JSHint contextual
dashboard #1684836: JSHint dashboard
edit [#] #1955292: JSHint edit
editor [#] #1955288: JSHint editor
field_ui #1751356: Selectors clean-up: field_ui #1684846: JSHint field_ui
file #1751382: Selectors clean-up: file module #1684848: JSHint file
filter #1751388: Selectors clean-up: filter module #1684850: JSHint filter
locale #1751394: Selectors clean-up: locale module #1684852: JSHint locale
menu #1751398: Selectors clean-up: menu module #1684854: JSHint menu
node #1751406: Selectors clean-up: node module #1684856: JSHint node
openid #1751408: Selectors clean-up: openid module #1684858: JSHint openid
overlay #1751412: Selectors clean-up: overlay module #1684862: JSHint overlay
path [#]
picture [#]
shortcut #1304486: Completely remove the ability to limit the number of shortcuts per set #1684866: JSHint shortcut
simpletest #1751418: Rewrite simpletest.js #1684868: JSHint simpletest
statistics #1684968: More reliable statistics.js #1684870: JSHint statistics
system #492720: Refactor system.js file and use Drupal form states and form ajax #1684872: JSHint system
taxonomy #1751430: Selectors clean-up: taxonomy module #1684874: JSHint taxonomy
text #1751346: Selectors clean-up: field/modules/text #1684842: JSHint field
toolbar #1686256: Refactor toolbar.js use localStorage remove cookie and no-js fallback #1684876: JSHint toolbar
tour #1938032: Simplify tour.js #1999806: JSHint tour
translation [#]
translation_entity [#] #1955286: JSHint translation_entity
user #1751434: Selectors clean-up: user module #1684880: JSHint user
views_ui #1839130: Refactor modules/views_ui/js/views-admin.js #1835102: JSHint modules/views/views_ui/js/views-admin.js
modules/views/ js/ajax.js [#] #1833050: JSHint modules/views/js/ajax.js
js/ajax_view.js [#] #1835074: JSHint modules/views/js/ajax-views.js
js/base.js #1835112: Clean-up modules/views/js/base.js #1835076: JSHint modules/views/js/base.js
js/jquery.ui.dialog.patch.js [#] #1835084: JSHint modules/views/js/jquery.ui.dialog.patch.js
js/view-contextual.js [#] #1835086: JSHint modules/views/js/views-contextual.js
themes/ bartik/color #1751436: Selectors clean-up: bartik/color theme #1684882: JSHint bartik
seven/js Vanilla JS! already ok

Comments

ksenzee’s picture

Discussed this some on IRC and everything currently in the issue description sounds great to me, and I think (I hope) pretty unobjectionable.

gary4gar’s picture

tstoeckler’s picture

@gary4gar: Please don't post subscribe comments anymore. We have a big green button for that at the top now :)

gary4gar’s picture

I have found eval on line 363 of ajax.js,

var progressBar = new Drupal.progressBar('ajax-progress-' + this.element.id, eval(this.progress.update_callback), this.progress.method, eval(this.progress.error_callback));

Now,so have two questions

1) how do we remove eval here? I think we can pass these as anonymous functions because functions are first-class citizens in javascript.
2) the reason I am asking this instead of doing it myself is because I have no way to try it out. so more importantly, How do test if changes I make in Drupal's javascript works or not?

Please, excuse my noobisness. I am quite new to drupal.

@tstoeckler
I have removed my comment & would use the follow button from next time :)

gary4gar’s picture

Issue summary:View changes

Avoid some bikeshedding

nod_’s picture

Title:Javascript cleanup» [Meta] javascript cleanup

hey gary4gar, I was going to use this issue for meta talk, each dot in the list should/will get it's own issue I made one for eval() removal. If you can't test it, don't change it. Someone else will be able to and will certainly help you out on the testing protocol.

if anyone wants to take up another task just create a new issue and link to it in here, i'll update the list.

nod_’s picture

Issue summary:View changes

link issue

nod_’s picture

Issue summary:View changes

add jquery task

nod_’s picture

Issue summary:View changes

add a related list

nod_’s picture

Issue summary:View changes

messed up previous edit

nod_’s picture

I'd like to add replacing $('selector', domContext) with $(domContext).find('selector') too.

The first one is expanded to the second internally by jQuery, it saves a function call and makes it more readable, it's closer to how we're actually traversing the DOM tree. I just want to check we're agreeing before adding to the todo list.

nod_’s picture

Ended up just doing it, please help review this big big patch #1419968: Replace $('selector', domelement) with $(domelement).find('selector').

nod_’s picture

Issue summary:View changes

Updated issue summary.

nod_’s picture

Issue summary:View changes

new task

nod_’s picture

Issue summary:View changes

add ajax.js issue

nod_’s picture

Issue tags:+JavaScript clean-up

as suggested by sun, adding new tag.

droplet’s picture

droplet’s picture

Issue summary:View changes

wrong issue number

nod_’s picture

Issue summary:View changes

move selector away from main list

nod_’s picture

Issue summary:View changes

add the selector issue back

nod_’s picture

Issue summary:View changes

link to issues

nod_’s picture

Added a couple of issues, no patch for all of them yet so feel free to take care of some if you're in the mood.

nod_’s picture

Issue summary:View changes

link to new issue

nod_’s picture

Issue summary:View changes

add new issues

droplet’s picture

#1428534: Use scrict equals,
#1460022: Prefix all jQuery variables with $

I'm thinking .... if split them into each module a issue thread that may work more efficiently ?

we can fix all performance and standard problems.

nod_’s picture

What I got from irc when I asked was that we do 1 small patch on 1 file, get that review, get that committed and followup with the big patch changing everything.

And I wouldn't start on the $ patch just yet, wait a week or two first :D

nod_’s picture

nod_’s picture

Issue summary:View changes

use strict

nod_’s picture

Title:[Meta] javascript cleanup» [Meta] javascript spring clean-up

updated summary

sun’s picture

I'm missing an issue for replacing context with $context.

I'm passing $context to custom behaviors in Dreditor, and I do not understand why we're passing the native DOM context instead of a jQuery $context in core.

sun’s picture

Issue summary:View changes

better presentation

nod_’s picture

Issue summary:View changes

add selectors clean-up

klonos’s picture

nod_’s picture

Title:[Meta] javascript spring clean-up» [Meta] javascript summer clean-up

Let's just hope we don't have to make it to winter.

The big table in the issue summary needs to be full of issues, ping me on irc or mail if you need help getting started with something. There are quite easy clean-ups to be taken care of for people starting with JS :)

nod_’s picture

Issue summary:View changes

Add oversized table

nod_’s picture

Issue summary:View changes

add ie8 css/js issues

nod_’s picture

Issue summary:View changes

issues in table

nod_’s picture

Issue summary:View changes

jshint to table

betoscopio’s picture

Hi nod_, i'm interested on those easy JS clean-ups, can be a good start to dive into this issue. There is a tag that flag this kind of issues?

nod_’s picture

oh yes, actually there is a tag: js-novice or you can also help review the JavaScript clean-up queue.

Ping me on IRC if you need some help :)

nod_’s picture

Issue summary:View changes

jshint

nod_’s picture

Issue summary:View changes

js folder

nod_’s picture

Issue summary:View changes

add jshint cleanup issues

nod_’s picture

Huge update on the JSHint issues. Most of them are either verified, RTBC or with a patch adressing most of the warnings.

nod_’s picture

Issue summary:View changes

file removed

nod_’s picture

Issue summary:View changes

promote jshint config

nod_’s picture

Issue summary:View changes

Add some refactor issues

nod_’s picture

Issue summary:View changes

autocomplete issue for D8

nod_’s picture

Issue summary:View changes

shortcut issue

nod_’s picture

Issue summary:View changes

add statistics issue

nod_’s picture

Issue summary:View changes

Change first contact to me for IRC help

nod_’s picture

Issue summary:View changes

Remove issue from list that is already in the table

nod_’s picture

Issue summary:View changes

Reorder issue lists for clearer view of what's left

nod_’s picture

Issue summary:View changes

add machine name refactor issue

nod_’s picture

Issue summary:View changes

toolbar issue

pascalduez’s picture

nod_’s picture

update the summary :)

nod_’s picture

Issue summary:View changes

add undeclared var issue

Jelle_S’s picture

Jelle_S’s picture

Issue summary:View changes

Added new issue #1749782

Jelle_S’s picture

Issue summary:View changes

Updated issue summary.

Jelle_S’s picture

Issue summary:View changes

Updated issue summary.

Jelle_S’s picture

Issue summary:View changes

Updated issue summary.

Jelle_S’s picture

Issue summary:View changes

Updated issue summary.

pascalduez’s picture

Issue summary:View changes

Added more js-cleaning issues to the table

pascalduez’s picture

Issue summary:View changes

Typo

pascalduez’s picture

Issue summary:View changes

Added more selectors cleaning issues.

pascalduez’s picture

Issue summary:View changes

Added more selectors cleaning issues.

nod_’s picture

Issue summary:View changes

right issue for system.js

seutje’s picture

nod_’s picture

Issue summary:View changes

add couple issues

nod_’s picture

hi there, can the people working on the selector/refactor issues have a look at this issue #1342198: Use .on and .off instead of .bind, .unbind and .delegate and take what's relevant for them? cosmicdreams did a great job of rerolling the patch with so many changes that got in lately. It'll help keep things revieable, thanks!

Kiphaas7’s picture

Kiphaas7’s picture

Issue summary:View changes

better spot

nod_’s picture

Issue summary:View changes

move dependency patch to done

Kiphaas7’s picture

Regarding #25: nod_ indicated that he rather see this added to individual issues than yet another meta issue. So just as an FYI if I start updating issue summaries.

nod_’s picture

Title:[Meta] javascript summer clean-up» [Meta] javascript autumn clean-up

Almost there :)

nod_’s picture

Issue summary:View changes

Add tableheader follow-up.

nod_’s picture

Added dropbutton and tableresponsive to the table.

nod_’s picture

Issue summary:View changes

add dropbutton and tableresponsive

nod_’s picture

Issue summary:View changes

add progress improvement issue.

nod_’s picture

Views is in core, so is it's JS. needs the follow-up issues created. There are issues with the jshint validation on the views files.

nod_’s picture

Issue summary:View changes

Add views JS

rballou’s picture

Issue summary:View changes

Added views JS issue numbers for JSHint

nod_’s picture

Issue summary:View changes

add matchmedia

rballou’s picture

Issue summary:View changes

Added issue number for final views JS file

nod_’s picture

Issue summary:View changes

add clean-up of base.js

nod_’s picture

Issue summary:View changes

add views-admin.js

nod_’s picture

Title:[Meta] javascript autumn clean-up» [Meta] javascript spring clean-up

Updated the table with all the JS stuff that got in since last time (ckeditor, edit, editor, tour, seven, picture, translation, translation_entity, path, debounce.js, dialog.js)

And yeah, I just skipped winter because winter is no fun.

droplet’s picture

about the debounce fucntion:
#1889394: Choose one JS debounce function

droplet’s picture

Issue summary:View changes

Big update

nod_’s picture

Issue summary:View changes

add tour issue

nod_’s picture

Updated the jshint issues, a few regressions.

nod_’s picture

Issue summary:View changes

updated jshint issues.

nod_’s picture

Issue summary:View changes

add couple issues

nod_’s picture

Issue summary:View changes

move done issues to proper place

nod_’s picture

In one month we're back to summer clean-up :)

Because of JSHint config changes #1995996: Update JSHint configuration a few JSHint issues will be reopened. Don't forget to update JSHint to the latest version.

nod_’s picture

Issue summary:View changes

update issues, add drop IE8 meta

klonos’s picture

...in 10 days actually :D

hass’s picture

Exists there any case that moves all .js files in modules to /js subfolder of the modules or this not planed? I've seen it only for CSS files and modules are very inconsistent.

hass’s picture

#1663622: Change directory structure for JavaScript files is only about /misc folder and introduces another inconsistency named /assets.

I'm more asking about toolbar.js, block.js and others located in module folders.

nod_’s picture

Feel free to open an issue about it. I agree but I haven't tried enforcing that yet.

nod_’s picture

Issue summary:View changes

Add a few JSHint issues and

nod_’s picture

Core passes JSHint validation!

nod_’s picture

Issue summary:View changes

close jshint column

nod_’s picture

Title:[Meta] javascript spring clean-up» [Meta] javascript winter clean-up
Issue summary:View changes

Alas, still not done.

I guess it's time to take a serious look at the coding standards. Can people have a look at #1778828: [policy, no patch] Update JS coding standards and help out there? thanks.

klonos’s picture

[#7230352] ???

nod_’s picture

Issue summary:View changes

Oups, fixed :)

LewisNyman’s picture

Issue tags:+frontend
xjm’s picture

So this issue mostly seems to be a tracking issue for a variety of JS work. As normal tasks, the child issues all need to be evaluated on whether they're still doable during the beta, and if appropriate, postponed to a later version. Thanks!