search

jashkenas / backbone

Give your JS App some Backbone with Models, Views, Collections, and Events
http://backbonejs.org
MIT License
28.09k stars 5.39k forks source link

Attempt to clean things up some #4193

Closed unquietwiki closed 6 years ago

unquietwiki commented 6 years ago

Hey @jashkenas . I've been working on a ES5->ES2015 project conversion; and have stumbled into trying to cleanup & assure that the assorted dependencies are reliable. I already have a pull in with Mediator; I figured this should get some similar attention. If this works, maybe it could be the starting point to resolve tickets like #3560 & #3924

Thanks!

unquietwiki commented 6 years ago

I snipped out the relevant failed tests from the Travis log

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection preinitialize FAILED
    Expected: 1
    Actual: undefined
    test/collection.js:672:17

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection preinitialize occurs before the collection is set up FAILED
    Expected 2 assertions, but 1 were run
    test/collection.js:675:13
    global code@test/collection.js:2103:3

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection mixin FAILED
    Died on test #1 test/collection.js:714:13
    global code@test/collection.js:2103:3: undefined is not a function (evaluating 'Backbone.Collection.mixin')
    test/collection.js:715:30

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection Collection.values iterates models in sorted order FAILED
    Died on test #1 test/collection.js:1797:13
    global code@test/collection.js:2103:3: undefined is not a function (evaluating 'collection.values()')
    test/collection.js:1803:37

    Expected 4 assertions, but 1 were run
    test/collection.js:1797:13
    global code@test/collection.js:2103:3

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection Collection.keys iterates ids in sorted order FAILED
    Died on test #1 test/collection.js:1810:13
    global code@test/collection.js:2103:3: undefined is not a function (evaluating 'collection.keys()')
    test/collection.js:1816:35

    Expected 4 assertions, but 1 were run
    test/collection.js:1810:13
    global code@test/collection.js:2103:3

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection Collection.entries iterates ids and models in sorted order FAILED
    Died on test #1 test/collection.js:1823:13
    global code@test/collection.js:2103:3: undefined is not a function (evaluating 'collection.entries()')
    test/collection.js:1829:38

    Expected 4 assertions, but 1 were run
    test/collection.js:1823:13
    global code@test/collection.js:2103:3

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Collection get models with `attributes` key FAILED
    failed, expected argument to be truthy, was: undefined
    Expected: true
    Actual: undefined
    test/collection.js:2101:14

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Events #3611 - listenTo is compatible with non-Backbone event libraries FAILED
    Died on test #1 test/events.js:706:13
    global code@test/events.js:743:3: undefined is not a function (evaluating 'this.events[name]()')
    trigger@test/events.js:714:26
    test/events.js:719:18

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Events #3611 - stopListening is compatible with non-Backbone event libraries FAILED
    Expected: 0
    Actual: 1
    test/events.js:741:17

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Model preinitialize FAILED
    Expected: 1
    Actual: undefined
    test/model.js:76:17

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Model preinitialize occurs before the model is set up FAILED
    Expected 6 assertions, but 3 were run
    test/model.js:80:13
    global code@test/model.js:1470:3

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Model mixin FAILED
    Died on test #1 test/model.js:1398:13
    global code@test/model.js:1470:3: undefined is not a function (evaluating 'Backbone.Model.mixin({
          isEqual: function(model1, model2) {
            return _.isEqual(model1, model2.attributes);
          }
        })')
    test/model.js:1399:25

    PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Router preinitialize FAILED
    Expected: "foo"
    Actual: undefined
    test/router.js:191:17

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.Router #4025 - navigate updates URL hash as is FAILED
    Expected: "#search/has%20space"
    Actual: "#search/has space"
    test/router.js:1078:23

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.View preinitialize FAILED
    Expected: 1
    Actual: undefined
    test/view.js:72:23

PhantomJS 2.1.1 (Linux 0.0.0) Backbone.View preinitialize occurs before the view is set up FAILED
    Expected 2 assertions, but 1 were run
    test/view.js:75:13
    global code@test/view.js:516:3
jashkenas commented 6 years ago

It looks like there are a lot of unrelated automatic style changes in this PR. Don't you think the meaningful changes should be separated out from stylistic things to make this easier to assess?

unquietwiki commented 6 years ago

Hey there. So the goal here was to use StandardJS rules to make the code more readable; and identify basic errors. The other major change was fixing up the testing system to account for newer browsers & NodeJS. This allowed for the creation of the bug report.

On May 4, 2018 4:48 PM, "Jeremy Ashkenas" notifications@github.com wrote:

It looks like there are a lot of unrelated automatic style changes in this PR. Don't you think the meaningful changes should be separated out from stylistic things to make this easier to assess?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/jashkenas/backbone/pull/4193#issuecomment-386761781, or mute the thread https://github.com/notifications/unsubscribe-auth/AA9fv1TGzGOG269_SVTA9IizxCaIwKkCks5tvOjHgaJpZM4TzYR1 .

unquietwiki commented 6 years ago

@paulfalgout Good morning! I dunno if you saw it, but over the weekend I did try to address your concerns regarding the PR. I hope they helped. Sorry if I wasn't as compliant Friday night: I was exhausted.

unquietwiki commented 6 years ago

@jashkenas @paulfalgout Whether this particular PR gets rejected / reworked / whatnot; I do have interest in getting some of these libraries updated to ES2015+ syntax. I've had to crash-course getting up to speed on JS in the past few months; and this is right when Mozilla is finishing up some key ES2015+ support for public use this week. The app I've been working on is 5+ years old (pre-2015); and the things I've found in trying to make that work, appear to solve a lot of old problems. I view such effort not unlike office filing: tedious, but necessary. Babel & Browserfy also seem useful in allowing for the source to be ES2015+; but still have production builds that'll run on ES5 runtimes.

unquietwiki commented 6 years ago

@paulfalgout @jashkenas I'll close this PR & do a revised one as able. I figure you might appreciate one that has the unit testing cleaned up, but the base code "virgin" to be less disruptive? Due apologies for any trouble on all of this.