JS error on latest master

[nemesifier]

Once on b8e8326, if I open the widget in the admin, the sortable feature is broken and I see the following errors in the JS console:

jquery-ui.js:6 Uncaught TypeError: Cannot read property 'ui' of undefined
jquery.js:4055 Uncaught TypeError: ul.sortable is not a function

If I reset to the previous commit I don't see the error, hence I think it was inadvertently introduced in #170.

@amirouche @clintonb @auvipy is anybody getting this error? Have you tried the latest master in one of your applications?

[amirouche]

The very first error comes from jquery-ui.js it is probably linked to the fact that there is several jquery.js or several jquery-ui.js that bring some trouble.

Can you change that line:

django-sortedm2m/sortedm2m/static/sortedm2m/widget.js

Line 149 in b8e8326

})(typeof jQuery === 'undefined' ? django.jQuery : jQuery);

And replace the ternary operation with simply django.jQuery?

Otherwise, if it does not work, you will need to provide an example code that reproduce the problem.

Have you tried the latest master in one of your applications?

Yes.

[nemesifier]

@amirouche, that didn't work.

Reintroducing these lines which were removed fixes it: b8e8326#diff-1a1e3ea5b934607f6eeba01fbf28dff55ed43758ded45b83c1b89e44c26effa5L1-L3

In order to replicate the issue, these instructions can be followed:
https://github.com/openwisp/openwisp-controller#installing-for-development

Then, make sure the latest master of django-sortedm2m is installed in the virtual env.

Then, the error shows up in http://localhost:8000/admin/config/device/add/.
It does show up also in the edit page, although a device configuration (inline object) and some templates need to be created for the widget to show up.

[amirouche]

The lines removed were replaced by the ternary operator at the end, which seems to me like the correct idiom, that I picked up in Django or something like that.

In order to replicate the issue, these instructions can be followed:
https://github.com/openwisp/openwisp-controller#installing-for-development

Then, make sure the latest master of django-sortedm2m is installed in the virtual env.

I will look into this tomorrow.

[nemesifier]

The lines removed were replaced by the ternary operator at the end, which seems to me like the correct idiom, that I picked up in Django or something like that.

Yes but jQuery ui expects to find a global jQuery object, which now is not available. Are you sure you're not getting the same issue in your application? If you're not seeing this issue, I suggest to double check if you don't have another line of JS somewhere defining a global jQuery object.

In the meanwhile I have opened #176, which I think it's the safest choice for maintaining backward compatibility and avoid problems in existing applications using this package.

[amirouche]

Yes, I have another jQuery in the global namespace. I disagree with the backward compatibility thing. This is a bug, not a feature.

[nemesifier]

Yes, I have another jQuery in the global namespace. I disagree with the backward compatibility thing. This is a bug, not a feature.

The solution used previously was not elegant but if we don't want to break the package we'll either have to find a better solution or rollback that change. I have not strong opinions, I only want this widget to keep working as it did in the last version.

[amirouche]

I do not have the time to investigate further.

Apprantly you rely on django-sortedm2m to have jQuery, that is a mistake.

[nemesifier]

I do not have the time to investigate further.

Apprantly you rely on django-sortedm2m to have jQuery, that is a mistake.

I do not rely, it's the jquery-ui shipped in django-sortedm2m which expects a global jQuery.

At the moment the latest master of this package seems broken to me, I highly advise to not release it to pypi until the problem is fixed to avoid breaking existing applications that rely on it.

[amirouche]

Sorry for the noise, I completely forgot sortedm2m rely on jqueryui!

Indeed #176 must be merged before a release.