Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

jQuery clashes #535

Closed
aaugustin opened this Issue · 17 comments

9 participants

@aaugustin
Owner

I'm creating a new issue to track in a single place all the problems related to the way the Debug Toolbar inserts jQuery into the page.

#440 was supposed to fix the problem, but it didn't work for the trivial case of an HTML page without any JavaScript.

I went for a very dumb solution in 2b6f5d6. I don't know any better and I'm unable to assess the solutions that have been offered.

As a consequence, until the other maintainers have a look at this issue or the toolbar gains new maintainers, this issue is unlikely to move forwards.

I will now close all the issues that are variations of this issue as duplicates.

@BertrandBordage

What about embedding jQuery inside a variable? Django admin does this, it then accesses jQuery using django.jQuery instead of jQuery or $. I know that supposes having jQuery loaded twice most of the time, but that's only for debug…

@aaugustin
Owner

Yes, that would make sense.

How would it interact with the AMD stuff?

@djfarrelly

The modified jQuery would have to have a different name also in it's define call. ex: define('toolbar.jquery') to make sure it doesn't clash with other versions of jQuery. In essence, changing the variable is very similar to what was in place before #440, with jQuery.noConflict(true) still functioning.

Note: This wouldn't fix the issue with AMD referenced in #506, that's a separate issue from this one.

@carltongibson

I quite like the solution in 2b6f5d6. I worry that other solutions are trying to do too much.

How about a setting, defaulting to True, USE_DEBUG_TOOLBAR_PROVIDED_JQUERY or such? Developers could turn this off if/when they're bundling their own jQuery (however they choose to do that).

(I wonder about overriding per-view, but I suspect that's too clever too.)

@maikhoepfel maikhoepfel referenced this issue in django-oscar/django-oscar
Closed

Number form fields continue to increase in Chrome #1131

@jeffbowen

+1 to having DDT either have a uniquely named version of jQuery (like @djfarrelly suggests) or a setting to disable DDT from including jQuery (like @carltongibson suggests). The way it works now is causing conflicts when I try to use RequireJS.

@stebunovd

+1 to uniquely named version of jQuery. This solution seem to be less error prone, because DDT will always use the same known version of jQuery. Use of external jQuery may potentially lead to issues related with incompatible jQuery version.

@aaugustin
Owner

@jeffbowen Could you explain "is causing conflicts"?

@stebunovd How would that interact with AMD?

It find it extremely depressing that all your comments sound there's a very easy solution, yet no one submits a pull request or even a code sample. I know I'm dumb, especially when it comes to frontend stuff. Will anyone please help? Can you all assume I have never written a single line of JavaScript when you talk to me?

Reading carefully your comments, I suspect that:

  • half of you are trying to use RequireJS and would like the DDT to stop inserting jQuery into the page;
  • the other half aren't using RequireJS and wondering why I'm bothering with this mess (I'm wondering too).

From now on, f you want to make an useful comment on this ticket, please consider both use cases. Thank you.


I spent some time this morning trying to write a patch to use a uniquely named version of jQuery, but I can't figure out how it's supposed to work with AMD.

Here's where I am:

<script src="//ajax.googleapis.com/ajax/libs/jquery/2.1.0/jquery.min.js"></script>
<script>var djdt = {}; djdt.jQuery = jQuery.noConflict(true);</script>

--> This loads a separate jQuery and makes it available in djdt.jQuery. We should take care to preserve it when overriding the global djdt variable in toolbar.js. But is it compatible with AMD / RequireJS?

Then each toolbar.<panel>.js file contains this piece of code:

(function (factory) {
    if (typeof define === 'function' && define.amd) {
        define(['jquery'], factory); // what does 'jquery' refer to here?
    } else {
        factory(jQuery);  // should be changed to djdt.jQuery
    }
}(function ($) {

    // code here 

}));

--> Can anyone answer the question I added in comment? How can I test that it works as expected?


tl;dr What's the proper implementation of RequireJS in our use case? Does anyone on Earth actually know how AMD and RequireJS work? Should I simply revert everything @singingwolfboy did and declare AMD and RequireJS as unsupported?

@robhudson @jezdez @dcramer Do you have any insights on this issue? I'm feeling extremely lonely...

@aaugustin aaugustin referenced this issue from a commit in aaugustin/django-debug-toolbar
@aaugustin aaugustin Always load a separate copy of jQuery.
I have no idea what this does for AMD / RequireJS users. No one is able
to explain to me how AMD / RequireJS work. I hope this commit doesn't
make the situation significantly worse, but really I can't tell.

Fix #535. (Well, not really, but it'll allow me to close the ticket.)
19514b0
@BertrandBordage BertrandBordage referenced this issue from a commit in BertrandBordage/django-debug-toolbar
@BertrandBordage BertrandBordage Adds another missing `djdt.jQuery`.
@aaugustin: thanks for tackling #535!  This last `djdt.jQuery` was missing.
I don't know how it interacts with #472, but this fixed #508 for me.
47c0df8
@BertrandBordage BertrandBordage referenced this issue in aaugustin/django-debug-toolbar
Closed

Adds another missing `djdt.jQuery`. #1

@stebunovd
define(['jquery'], factory); // what does 'jquery' refer to here?

jquery here refers to jQuery registered in RequireJS configuration. Example from RequireJS docs:

require.config({
    baseUrl: 'js/lib',
    paths: {
        // the left side is the module ID,
        // the right side is the path to
        // the jQuery file, relative to baseUrl.
        // Also, the path should NOT include
        // the '.js' file extension. This example
        // is using jQuery 1.9.0 located at
        // js/lib/jquery-1.9.0.js, relative to
        // the HTML page.
        jquery: 'jquery-1.9.0'
    }
});

As you can see, in this case jquery will refer to the file located at js/lib/jquery-1.9.0.js

@stebunovd

If you wish I can make a sample Django project for you which uses RequireJS, so you can use it for testing

@djfarrelly

@aaugustin to try to help explain, RequireJS's define function is used to register a module with an export value. jQuery's source includes a block of code that detects if the page is using RequireJS (or another AMD loader) and it calls the define function to register the 'jquery' namespace. Here is how an issue could exist:

User's project uses RequireJS and jQuery 1.7 which contains the define call in it's source like:

define( 'jquery', function(){ return jQuery; });

User's project consumes jQuery as a dependency in their web application like this:

// 'jquery' here is a dependency that matches the 'jquery' above
// that "defined"/registered the module within RequireJS
require([ 'jquery' ], function($){ /* some code that needs jQuery 1.7 */ });

DjDT adds jQuery to the page. RequireJS only allows 1 module named 'jquery' to be registered:

// within the jQuery 2.1 source:
define('jquery', function($){  return jQuery;  });

DjDT's toolbar.js consumes 'jquery' as a dependecy (All dependencies in define calls are within an Array). jQuery is passed as an argument to the factory function:

define(['jquery'], factory);

The factory function is expecting jQuery 2.1, but since 'jquery' was defined first the jQuery 1.7 file, toolbar.js gets jQuery 1.7 instead.


To correct any potential issues we would have to modify a version of jQuery bundled with DjDTand create a custom jQuery namespace, ex. "djdt.jquery":

// within the jQuery 2.1 source:
define('djdt.jquery', function($){  return jQuery;  });

Now, toolbar.js can consume it's dependency, jQuery, knowing that it won't conflict with any other potential version that was loaded earlier in the page:

// 'djdt.jquery' is the dependency required, which matches our modified jQuery 2.1
define(['djdt.jquery'], factory);

I hope I was able to explain this a little bit. RequireJS can be tricky to wrap your head around if you've never used it before. My PR #544 builds on #541 to add the jQuery namespace so DjDT's code uses's it's version of jQuery defined within the jQuery source just like the example above.

@aaugustin
Owner

Yes, it's much more clear for me with this explanation. I wasn't able to get that from the official docs. Thank you very much. I'll play with RequireJS and review your proposal.

@jeffbowen

@aaugustin Sorry for the vagueness in my previous comment. @djfarrelly's last comment does a great job of explaining my problem. I'm new to RequireJS as well. Really appreciate your work on this! I love DDT.

@aaugustin aaugustin referenced this issue from a commit in aaugustin/django-debug-toolbar
@aaugustin aaugustin Always load a separate copy of jQuery.
I have no idea what this does for AMD / RequireJS users. No one is able
to explain to me how AMD / RequireJS work. I hope this commit doesn't
make the situation significantly worse, but really I can't tell.

Fix #535. (Well, not really, but it'll allow me to close the ticket.)
8efd103
@jmbowman

What I had to do to get version 1.0.1 working in a site that uses RequireJS:

  1. Overrode debug_toolbar/base.html in one of my apps with a copy that doesn't include any script tags (having a setting in DEBUG_TOOLBAR_CONFIG like AUTO_LOAD_JAVASCRIPT which defaults to True but would omit these tags when set to False would be nice)

  2. In a context processor, added the result of calling SHOW_TOOLBAR_CALLBACK to a DEBUG_TOOLBAR context variable

  3. In my site's base template, added an {% if DEBUG_TOOLBAR %} section which loads toolbar.js in a manner consistent with the way other scripts in my site are loaded (the site already had compatible versions of jquery and jquery.cookie)

The details of step 3 can vary a fair bit between different sites, so I think it's best to just assume that somebody who's gone to the effort of configuring RequireJS or another AMD loader can figure this out; I can provide the exact code I used if people are interested. Even if the site uses a version of jQuery that doesn't work with the toolbar, I think it's better for the site developer to sort that out than to have the toolbar try to automagically solve it somehow (RequireJS has some nice options for loading multiple versions of the same library, but it really needs to be done in the main configuration script). The main thing is to document which script(s) need to be loaded for the toolbar to work.

@arcanis

The thing I don't understand is how you manage you avoid issues related to

Mismatched anonymous define() module

The code is currently using anonymous define() inside a <script/> tag, instead of regular require(). Such a thing should (and, in my case, does) trigger a bunch of require.js errors (since require.js cannot give a name based on the script path to file loaded using tags).

Also, I think a correct way to implement dependencies is to set a custom dependency name, ie :

require( [
    'djdt.jquery'
], function ( $ ) {
    // ...
} )

Then the user just has to chose the correct JQuery version using the following configuration :

require.configuration( {
    map : {
        '*' : {
            'djdt.jquery' : 'jquery-2.X'
        }
    }
} );

And you can even catch the 'does not exists' event and load another file if you wish to.

Note that this snippet does not require any custom JQuery version, which would be very bad anyway.

@jmbowman

I worked around the "Mismatched anonymous define() module" error by loading the script via a require() call in another script instead of directly in an HTML script element...but yes, that needs to be a require() instead of a define() otherwise. The same change needs to be made in the panel scripts for them to load correctly in an AMD environment:

  • toolbar.profiling.js
  • toolbar.sql.js
  • toolbar.template.js
  • toolbar.timer.js

Even if the jQuery dependency is renamed, I'd prefer to have the option of skipping the scripts in the template in favor of loading them myself (my site is structured such that the requirejs configuration call typically hasn't completed yet by the time those scripts are added to the page, so they typically fail to load with path errors).

@AdamCraven

I've just started with Django/Python, so would be uncomfortable creating a well-crafted pull request for this, but good on the front-end.

The solution I'd recommend is loading the django toolbar up in an iframe. This will isolate/sandbox it from the parent document and you won't have any conflicts anymore.

I've had a quick look in the source code, what I would suggest is create a new file called iframe.html, this should be the first file loaded. Then add to it: <iframe src="{% link to base.html %}"><\/iframe>

If you notice there, it isn't actually the content of the file itself, but a hyperlink to base.html, which would have to be parsed by python when the url loads.

Unfortunately the downside of this is the page will need to be restyled to fit back in with the way it is now and it won't look exactly the same to start off with. But much can be done.

Perhaps the interim solution is to allow users to use the iframe version as a configuration choice or in a branch and then someone (perhaps even myself) can restyle it to fit closer to the original design.

@djfarrelly

Hey @aaugustin, is there any reason why the toolbar couldn't be loaded in an iframe like @AdamCraven suggested? Or does it have to be within the same request as the page response to grab the data from it? It's probably the cleanest possible solution and DJDT would never have to worry about any conflicts.

If It needs data from the response itself, we could pass it through to the iframe via postMessage.

@aaugustin aaugustin referenced this issue from a commit
@aaugustin aaugustin Make the pain go away.
Fix #581, #544, #541, #535 and a few others.
727b4e0
@aaugustin aaugustin closed this
@mbarrien mbarrien referenced this issue from a commit in mbarrien/django-debug-toolbar
@mbarrien mbarrien Re-fix #535 fix reversed noConflict in DjDT jQuery include d69f493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.