{"action":"created","issue":{"url":"https://api.github.com/repos/cyclejs/cyclejs/issues/759","repository_url":"https://api.github.com/repos/cyclejs/cyclejs","labels_url":"https://api.github.com/repos/cyclejs/cyclejs/issues/759/labels{/name}","comments_url":"https://api.github.com/repos/cyclejs/cyclejs/issues/759/comments","events_url":"https://api.github.com/repos/cyclejs/cyclejs/issues/759/events","html_url":"https://github.com/cyclejs/cyclejs/pull/759","id":282451969,"number":759,"title":"Fix sibling isolation implementation in DOM driver","user":{"login":"jvanbruegge","id":1529052,"avatar_url":"https://avatars1.githubusercontent.com/u/1529052?v=4","gravatar_id":"","url":"https://api.github.com/users/jvanbruegge","html_url":"https://github.com/jvanbruegge","followers_url":"https://api.github.com/users/jvanbruegge/followers","following_url":"https://api.github.com/users/jvanbruegge/following{/other_user}","gists_url":"https://api.github.com/users/jvanbruegge/gists{/gist_id}","starred_url":"https://api.github.com/users/jvanbruegge/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jvanbruegge/subscriptions","organizations_url":"https://api.github.com/users/jvanbruegge/orgs","repos_url":"https://api.github.com/users/jvanbruegge/repos","events_url":"https://api.github.com/users/jvanbruegge/events{/privacy}","received_events_url":"https://api.github.com/users/jvanbruegge/received_events","type":"User","site_admin":false},"labels":[{"id":708626252,"url":"https://api.github.com/repos/cyclejs/cyclejs/labels/PR:%20awaiting%20review","name":"PR: awaiting review","color":"fcfc67","default":false}],"state":"open","locked":false,"assignee":{"login":"jvanbruegge","id":1529052,"avatar_url":"https://avatars1.githubusercontent.com/u/1529052?v=4","gravatar_id":"","url":"https://api.github.com/users/jvanbruegge","html_url":"https://github.com/jvanbruegge","followers_url":"https://api.github.com/users/jvanbruegge/followers","following_url":"https://api.github.com/users/jvanbruegge/following{/other_user}","gists_url":"https://api.github.com/users/jvanbruegge/gists{/gist_id}","starred_url":"https://api.github.com/users/jvanbruegge/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jvanbruegge/subscriptions","organizations_url":"https://api.github.com/users/jvanbruegge/orgs","repos_url":"https://api.github.com/users/jvanbruegge/repos","events_url":"https://api.github.com/users/jvanbruegge/events{/privacy}","received_events_url":"https://api.github.com/users/jvanbruegge/received_events","type":"User","site_admin":false},"assignees":[{"login":"jvanbruegge","id":1529052,"avatar_url":"https://avatars1.githubusercontent.com/u/1529052?v=4","gravatar_id":"","url":"https://api.github.com/users/jvanbruegge","html_url":"https://github.com/jvanbruegge","followers_url":"https://api.github.com/users/jvanbruegge/followers","following_url":"https://api.github.com/users/jvanbruegge/following{/other_user}","gists_url":"https://api.github.com/users/jvanbruegge/gists{/gist_id}","starred_url":"https://api.github.com/users/jvanbruegge/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jvanbruegge/subscriptions","organizations_url":"https://api.github.com/users/jvanbruegge/orgs","repos_url":"https://api.github.com/users/jvanbruegge/repos","events_url":"https://api.github.com/users/jvanbruegge/events{/privacy}","received_events_url":"https://api.github.com/users/jvanbruegge/received_events","type":"User","site_admin":false}],"milestone":null,"comments":16,"created_at":"2017-12-15T14:54:48Z","updated_at":"2018-03-28T14:24:06Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/cyclejs/cyclejs/pulls/759","html_url":"https://github.com/cyclejs/cyclejs/pull/759","diff_url":"https://github.com/cyclejs/cyclejs/pull/759.diff","patch_url":"https://github.com/cyclejs/cyclejs/pull/759.patch"},"body":"This PR will remove the use of CSS classes for sibling isolation.\r\nCurrently I just added a test that fails in the current implementation\r\n\r\nISSUES FIXED: #758, #587, #700, #243, #545\r\nNEEDS TEST: #333\r\nWORK IN PROGRESS: #775, #747\r\n\r\n# What the *fish* I did here:\r\n## Definitions:\r\n- A **namespace** is an Array of scopes.\r\n- A **scope** is an object with a type and a value\r\n- A scope **type** is either total, sibling or selector\r\n- A scope **value** can be anything serializable and is used to distinguish the scopes\r\n\r\n## Algorithm\r\n- Work is divided into two main parts: Isolation (IsolateModule) and event handling (EventDelegator)\r\n    * All event handling code was removed from the MainDOMSource\r\n- Isolate module expects **namespace** on vnode on create (only for isolation roots), saves element in tree.\r\n    * Top level is `[]` the root namespace without isolation.\r\n    * Traversing the tree and collecting the **scope values** gives back **namespace**\r\n    * Using a special Symbol you can get the Element of that isolation scope (to avoid collision with **scope value**)\r\n    * Namespace can be queried by looking for parent of Element in namespace tree\r\n- Event delegator adds a virtual listener in namespace tree\r\n    * total and sibling are saved as nodes in the tree, selector is saved extra\r\n    * has actual listeners at the root element for bubbling events\r\n    * non bubbling events are saved in a Map of Maps (eventType -> Element -> Listener)\r\n    * resets listener on element updates (avoids recreation)\r\n    * simulates capture phase with inverse bubbling path\r\n    * simulates bubble phase\r\n## Other stuff that also happened\r\n- No polyfills any more\r\n- More ES6 features used (Map, Set and Symbol vs. just Map)\r\n- No observable change to the vnodes (no additional css classes etc)\r\n## Why are the issues Fixed\r\n- #758: No css classes added to the elements (verified by test)\r\n- #587: Just one delegator at any time in the program\r\n- #700: Just deleted the polyfills\r\n- #243: Symbol usedTree to avoid name collision\r\n\r\n## What still needs work\r\n- [ ] #333: Add test to check non-bubbling listener cleanup\r\n- [ ] #775: Check if rxjs and most typings are up-to-date\r\n- [ ] #747: Add test for remove animatios\r\n- [ ] #540: Add test for manipulation of head/body/html\r\n- [ ] Check why **sometimes** `DOMSource.events() should catch non-bubbling events in a DocumentFragment with useCapture` and `isolation should handle events when child is removed and re-added` fail in older Chrome Browsers\r\n- [ ] Document the breaking changes\r\n\r\n<hr>\r\n\r\n- [x] There exists an issue discussing the need for this PR #758\r\n- [x] I added new tests for the issue I fixed or built\r\n- [x] I ran `make test FOO` for the package FOO I'm modifying\r\n- [x] I used `make commit` instead of `git commit`\r\n"},"comment":{"url":"https://api.github.com/repos/cyclejs/cyclejs/issues/comments/376905484","html_url":"https://github.com/cyclejs/cyclejs/pull/759#issuecomment-376905484","issue_url":"https://api.github.com/repos/cyclejs/cyclejs/issues/759","id":376905484,"user":{"login":"jvanbruegge","id":1529052,"avatar_url":"https://avatars1.githubusercontent.com/u/1529052?v=4","gravatar_id":"","url":"https://api.github.com/users/jvanbruegge","html_url":"https://github.com/jvanbruegge","followers_url":"https://api.github.com/users/jvanbruegge/followers","following_url":"https://api.github.com/users/jvanbruegge/following{/other_user}","gists_url":"https://api.github.com/users/jvanbruegge/gists{/gist_id}","starred_url":"https://api.github.com/users/jvanbruegge/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jvanbruegge/subscriptions","organizations_url":"https://api.github.com/users/jvanbruegge/orgs","repos_url":"https://api.github.com/users/jvanbruegge/repos","events_url":"https://api.github.com/users/jvanbruegge/events{/privacy}","received_events_url":"https://api.github.com/users/jvanbruegge/received_events","type":"User","site_admin":false},"created_at":"2018-03-28T14:24:06Z","updated_at":"2018-03-28T14:24:06Z","author_association":"MEMBER","body":"I took a look at the test and it's always the same tests. Out of 4 CI logs:\r\n| Count | Browser | Test Name |\r\n|--------|-----------|---------------|\r\n| 4 | Chrome 49.0 | DOMSource.events() should catch non-bubbling events in a DocumentFragment with useCapture |\r\n| 2 | Chrome 30.0 | isolation should handle events when child is removed and re-added |"}}
{"actor":{"display_login":"jvanbruegge"}}