Observe shadowRoot mutations #18

valdrinkoshi · 2016-08-06T00:35:20Z

Fixes #17 by observing node changes on the shadowRoot of each InertRoot.

If a node doesn't have yet a shadow root, we patch the methods to create a shadow root and we add our observer then! This might be slow, ideally we'd need to observe shadowRoot creation, but this is not possible yet in ShadowDOM v1 WICG/webcomponents#204

valdrinkoshi · 2016-08-06T00:40:48Z

@robdodson @alice FYI

valdrinkoshi · 2016-08-12T18:22:04Z

inert.js

+    const rootObserver = new MutationObserver(boundOnMutation);
+    const shadowRoot = this._rootElement.shadowRoot || this._rootElement.webkitShadowRoot;
+    if (shadowRoot) {
+      rootObserver.observe(shadowRoot, { childList: true, subtree: true });


memo to self: we should be observing also the attributes..!

@alice actually here we shoudn't observe attributes, as MutationObserver will observe the ones of the target. In this case, the target is the shadowRoot which won't get any attribute change. this._observer will take care of that.

valdrinkoshi · 2016-08-18T03:18:48Z

test/index.js

+        shadowButton.textContent = 'Shadow button';
+        host.shadowRoot.appendChild(shadowButton);
+        // Give time to mutation observers.
+        setTimeout(function() {


memo to self: use Promise instead of timeout

alice · 2016-10-26T17:48:43Z

inert.js

    this._observer = null;

+    this._rootObserver.disconnect();
+    this._rootObserver = null;


Should we un-monkeypatch createShadowRoot/attachShadow here as well?

Good point, will update!

valdrinkoshi · 2016-10-27T00:54:19Z

inert.js

+    this._rootElement.removeAttribute('aria-hidden');
+    // Restore original attachShadow and createShadowRoot.
+    delete this._rootElement.attachShadow;
+    delete this._rootElement.createShadowRoot;


This assumes that no-one else has patched these methods

valdrinkoshi · 2016-10-27T01:01:21Z

@alice @robdodson another approach could be to fire a shadow-root-created event on the patched attachShadow/createShadowRoot, since this seems to be in line with the plan for ShadowDOM v2 WICG/webcomponents#204.
That would translate in patching the HTMLElement.prototype.attachShadow/createShadowRoot and fire the event in the patched functions. The advantage is that when v2 is implemented, we can just remove the attachShadow patch while keeping the one for createShadowRoot.
WDYT?

valdrinkoshi · 2016-10-27T02:35:43Z

PS: when I run gulp I get this error:

$ gulp
[19:34:13] Using gulpfile ~/dev/inert/gulpfile.js
/Users/valdrin/homebrew/lib/node_modules/gulp/bin/gulp.js:129
    gulpInst.start.apply(gulpInst, toRun);
                  ^

TypeError: Cannot read property 'apply' of undefined
    at /Users/valdrin/homebrew/lib/node_modules/gulp/bin/gulp.js:129:19
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)
    at Module.runMain (module.js:609:11)
    at run (bootstrap_node.js:382:7)
    at startup (bootstrap_node.js:137:9)
    at bootstrap_node.js:497:3

@robdodson any idea why? I tried to follow this guide but no success https://gist.github.com/DanHerbert/9520689

robdodson · 2016-10-27T08:06:29Z

@valdrinkoshi which version of node and gulp are you running?

valdrinkoshi · 2016-10-27T18:28:33Z

@robdodson

valdrin-macbookpro:inert valdrin$ node -v
v7.0.0
valdrin-macbookpro:inert valdrin$ npm -v
3.10.9
valdrin-macbookpro:inert valdrin$ gulp -v
[11:28:03] CLI version 3.9.0
[11:28:03] Local version 4.0.0-alpha.2

robdodson · 2016-10-27T20:07:43Z

hm that seems fine... Personally I use nvm to manage my node versions (https://github.com/creationix/nvm) and haven't had issues...

valdrinkoshi · 2016-10-27T20:58:21Z

@robdodson could you share your node, npm, gulp versions?

robdodson · 2016-10-27T21:17:26Z

~/Desktop/poly-foo
❯ node -v
v6.3.0

~/Desktop/poly-foo
❯ npm -v
3.10.3

~/Desktop/poly-foo
❯ gulp -v
[14:17:16] CLI version 1.2.2

valdrinkoshi · 2016-10-27T22:14:48Z

Yay! made it! Had to update the gulp-cli.

Concerning this PR, it's ready for review 👌 I ended up preferring two methods to patch/unpatch attachShadow rather than firing an event, since I agree that firing an event when a shadow root is created kind of defeats the purpose of ShadowDOM (as commented in WICG/webcomponents#204): why should this be notified to the outside elements? it is useful only for tooling or polyfills - there's not a real use case.

robdodson · 2017-03-09T16:20:48Z

@alice maybe we can spend some time reviewing this in Sydney?

valdrinkoshi force-pushed the handle-shadow-roots branch 2 times, most recently from 3c0dba7 to 72d618f Compare August 6, 2016 00:38

valdrinkoshi changed the title ~~Handle shadow roots~~ Observe shadowRoot mutations Aug 6, 2016

valdrinkoshi force-pushed the handle-shadow-roots branch 3 times, most recently from aedca2f to 5834723 Compare August 8, 2016 20:59

valdrinkoshi reviewed Aug 12, 2016
View reviewed changes

valdrinkoshi force-pushed the handle-shadow-roots branch from cd883fb to 40a9a32 Compare August 16, 2016 19:55

valdrinkoshi mentioned this pull request Aug 17, 2016

Observe ShadowRoot attachment WICG/webcomponents#204

Closed

valdrinkoshi reviewed Aug 18, 2016
View reviewed changes

valdrinkoshi force-pushed the handle-shadow-roots branch from 40a9a32 to 6e625aa Compare August 30, 2016 20:37

valdrinkoshi force-pushed the handle-shadow-roots branch from 6e625aa to 075a579 Compare September 9, 2016 01:16

alice reviewed Oct 26, 2016

View reviewed changes

valdrinkoshi force-pushed the handle-shadow-roots branch from 075a579 to c600fc2 Compare October 27, 2016 00:52

valdrinkoshi commented Oct 27, 2016

View reviewed changes

valdrinkoshi force-pushed the handle-shadow-roots branch from 23ac4f3 to 80e2528 Compare October 27, 2016 02:19

valdrinkoshi force-pushed the handle-shadow-roots branch 3 times, most recently from 568bbfe to 1c53778 Compare November 23, 2016 19:14

observe shadow root child nodes

d698504

valdrinkoshi added 4 commits March 9, 2017 15:22

include link to issue on webcomponents

a3016bb

use promises to wait mutation observers

4cafe5c

restore original attachShadow/createShadowRoot on destructor

fbe5ebe

patch/unpatch methods

48353e2

valdrinkoshi force-pushed the handle-shadow-roots branch from 1c53778 to 48353e2 Compare March 9, 2017 23:31

yoavweiss changed the base branch from master to main March 5, 2021 21:37

Observe shadowRoot mutations #18

Are you sure you want to change the base?

Observe shadowRoot mutations #18

Uh oh!

Conversation

valdrinkoshi commented Aug 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valdrinkoshi commented Aug 6, 2016

Uh oh!

valdrinkoshi Aug 12, 2016

Choose a reason for hiding this comment

Uh oh!

valdrinkoshi Aug 16, 2016

Choose a reason for hiding this comment

Uh oh!

valdrinkoshi Aug 18, 2016

Choose a reason for hiding this comment

Uh oh!

alice Oct 26, 2016

Choose a reason for hiding this comment

Uh oh!

valdrinkoshi Oct 26, 2016

Choose a reason for hiding this comment

Uh oh!

valdrinkoshi Oct 27, 2016

Choose a reason for hiding this comment

Uh oh!

valdrinkoshi Oct 27, 2016

Choose a reason for hiding this comment

Uh oh!

valdrinkoshi commented Oct 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valdrinkoshi commented Oct 27, 2016

Uh oh!

robdodson commented Oct 27, 2016

Uh oh!

valdrinkoshi commented Oct 27, 2016

Uh oh!

robdodson commented Oct 27, 2016

Uh oh!

valdrinkoshi commented Oct 27, 2016

Uh oh!

robdodson commented Oct 27, 2016

Uh oh!

valdrinkoshi commented Oct 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robdodson commented Mar 9, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

valdrinkoshi commented Aug 6, 2016 •

edited

Loading

valdrinkoshi commented Oct 27, 2016 •

edited

Loading

valdrinkoshi commented Oct 27, 2016 •

edited

Loading