Classes split

[drzraf]

Split src/flow.js into distinct classes and rely upon build-time assembly.

• real class methods replace the former simili-constructor syntax
• FlowFile and evalOpts (counter-intuitive) bindings into Flow namespace had to be dirtily reproduced because the Grunt/Karma/Jasmine hardly support full ES6 import within spec files.
• Tests adapted
• flow.support removed
  1. Because if there is no window, user are on their own and there is no reason keeping node.js from loading the code
  2. Because support rather depend on the transpilation and polyfills part of the final build issues by the toolchain => Avoid cluttering bootstrap with a runtime check as for window/global check

The only really sensitive part was:

• $ shorthand symbol was removed in favor of this and context has been taken care of (including for event handlers)
• started replacing a couple each() by for..of when it helped getting rid of significant context switch complexity.

Based on https://github.com/flowjs/flow.js/pull/314

Note: I initially attempted to import tools.js straight within Jasmine but karma kept me from doing real ES6 imports there (sadly).

Note: One test fails in test/setupSpec.js because

flow.unAssignDrop(div);
div.dispatchEvent(event);

issued an onDrop event. but I couldn't figure what's going on since I basically didn't changed the code other than binding the necessary event handlers.

[AidasK]

Looks neat, you're awesome ❤️

Feel free to merge anything to v3 yourself :))

[drzraf]

Feel free to merge anything to v3 yourself :))

Ok. Do you think you may have a look at that test though?

[AidasK]

I have cloned this branch and every test passes

Running "karma:coverage" (karma) task
(node:87550) Warning: Accessing non-existent property 'VERSION' of module exports inside circular dependency
(Use `node --trace-warnings ...` to show where the warning was created)
17 09 2020 11:22:19.437:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
17 09 2020 11:22:19.441:INFO [launcher]: Starting browser Firefox
17 09 2020 11:22:23.306:INFO [Firefox 79.0.0 (Mac OS X 10.15.0)]: Connected on socket ceqhdUsmQmBr2EdiAAAA with id 8771550
Firefox 79.0.0 (Mac OS X 10.15.0): Executed 56 of 56 SUCCESS (0.137 secs / 0 secs)

Done, without errors.

Though saucelabs implementation seems to be broken, browser session starts though no commands are sent. You can still merge this and we will solve this saucelab issue later.

[drzraf]

Chrome Headless 84.0.4147.135 (Linux x86_64) setup assignDrop assign to div FAILED
    Error: Expected 2 to be 1.
        at <Jasmine>
        at UserContext.<anonymous> (test/setupSpec.js:115:36)
        at <Jasmine>
Chrome Headless 84.0.4147.135 (Linux x86_64): Executed 55 of 55 (1 FAILED) (0.276 secs / 0.153 secs)
Firefox 81.0 (Ubuntu 0.0.0) setup assignDrop assign to div FAILED
    Expected 2 to be 1.
    <Jasmine>
    @test/setupSpec.js:115:36
    <Jasmine>
Chrome Headless 84.0.4147.135 (Linux x86_64): Executed 55 of 55 (1 FAILED) (0.276 secs / 0.153 secs)
Firefox 81.0 (Ubuntu 0.0.0): Executed 55 of 55 (1 FAILED) (0.263 secs / 0.164 secs)

(I'm not able to use saucelab so I can't test on other FF versions)

[drzraf]

That was quite a tricky one, see fd0c1f71a0.

First, on the previous code, the function wasn't bound to this. I don't understand how onDrop used to work then.

But then, by using bind(this) is that a class which does: foo.addEventListener('drop', this.method.bind(this), false); can't remove said event-listener afterwards using: foo.addEventListener('drop', this.method.bind(this), false); because bind actually change the method. So a reference must be kept.

That's what fd0c1f7 does. (it also updates the way Jasmine spy must be setup).

[AidasK]

Awesome, good catch!