patternfly-bootstrap-treeview

patternfly-bootstrap-treeview copied to clipboard

Make the library compatible with jQuery v3.

Hmm, I am not against it, but we depend on this project in ManageIQ and this might break stuff. What do you think @himdel || @karelhala?

As long as "make compatible with jquery 3" doesn't mean "break compatibility with previous releases". :)

@akieling What's missing here is any kind of indication why this would not be compatible now, and what kind of changes you need done for it to be compatible.

(Frankly I'm surprised this is not compatible already, are we using some deprecated function somewhere?)

Well we are using >= for jQuery in bower.json#L29 so while testing it might download jQuery 3 without us knowing it. Because https://docs.npmjs.com/misc/semver, so same question as @himdel's are we using something from old jquery we shouldn't?

The tests are using jQuery v2. Have a look at ./tests/tests.html. It references ./tests/lib/jquery.js, not the one installed by Bower or npm. When you update it to v3, most tests break.

On 29 Jan 2018 11:40 am, "Karel Hala" [email protected] wrote:

Well we are using >= for jQuery in bower.json#L29 https://github.com/jonmiles/bootstrap-treeview/blob/master/bower.json#L29 so while testing it might download jQuery 3 without us knowing it. Because https://docs.npmjs.com/misc/semver, so same question as @himdel https://github.com/himdel's are we using something from old jquery we shouldn't?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/patternfly/patternfly-bootstrap-treeview/issues/65#issuecomment-361248047, or mute the thread https://github.com/notifications/unsubscribe-auth/AB0lT9hqpfkccV6ksSNgUC8_9eHAAZmjks5tPco-gaJpZM4RuW4_ .

Looks like the relevant failure is this one:

$ grunt test
Running "qunit:all" (qunit) task
Testing tests/tests.html .FFF.FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
>> PhantomJS timed out, possibly due to:
>> - QUnit is not loaded correctly.
>> - A missing QUnit start() call.
>> - Or, a misconfiguration of this task.
Warning: 1 tests completed with 1 failed, 0 skipped, and 0 todo. 
0 assertions (in 0ms), passed: 0, failed: 0 Use --force to continue.

that looks more like qunit isn't starting properly than actual failures.

Looks like the tests need to be fixed to use a current version of qunit and jquery, and not to bundle these things in the repo.

So far, I don't see any evidence of patternfly-bootstrap-treeview not working with jQuery 3.

Trying the demo, it seems to work, though I am getting this exception:

jquery.js:3818 jQuery.Deferred exception: Cannot read property 'length' of undefined TypeError: Cannot read property 'length' of undefined
    at Function.grep (http://localhost:3000/bower_components/jquery/dist/jquery.js:418:19)
    at Tree._findNodes (http://localhost:3000/js/bootstrap-treeview.js:1874:12)
    at Tree._getSearchResults (http://localhost:3000/js/bootstrap-treeview.js:1851:15)
    at Tree.search (http://localhost:3000/js/bootstrap-treeview.js:1800:23)
    at Object.proxy [as search] (http://localhost:3000/bower_components/jquery/dist/jquery.js:10268:13)
    at HTMLDivElement.<anonymous> (http://localhost:3000/js/bootstrap-treeview.js:1934:30)
    at Function.each (http://localhost:3000/bower_components/jquery/dist/jquery.js:354:19)
    at jQuery.fn.init.each (http://localhost:3000/bower_components/jquery/dist/jquery.js:189:17)
    at jQuery.fn.init.$.fn.(anonymous function) [as treeview] (http://localhost:3000/js/bootstrap-treeview.js:1921:8)
    at findSelectableNodes (http://localhost:3000/:571:34) undefined

... and that's probably a timing bug .. jQuery 3 changed their promise-like thing to behave according to Promise specs, meaning that done should never happen synchronously.

So presumably the this._orderedNodes = this._sortNodes() assignment is now happening later than that return $.grep(this._orderedNodes, $.proxy(function (node) {.

I'm getting the same error when upgrading one of the hawt.io projects to jQuery 3. Has anyone tried running the tests with the latest versions of jQuery and qunit? I can try that next week.

On 30 Jan 2018 11:24 am, "Martin Hradil" [email protected] wrote:

... and that's probably a timing bug .. jQuery 3 changed their promise-like thing to behave according to Promise specs, meaning that done should never happen synchronously.

So presumably the this._orderedNodes = this._sortNodes() assignment is now happening later than that return $.grep(this._orderedNodes, $.proxy(function (node) {.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/patternfly/patternfly-bootstrap-treeview/issues/65#issuecomment-361592096, or mute the thread https://github.com/notifications/unsubscribe-auth/AB0lT0QhV7kfikp_LQVKyLAJ5pnnz85Iks5tPxghgaJpZM4RuW4_ .

@akieling I.. tried to try :) But looks like the tests need a complete rewrite to work with current versions of qUnit.

If you can get them working, a PR would be much appreciated :). (Preferrably so that we don't have to bundle qunit and jquery in the repo anymore, but we can take care of that.)