Use CSS keyframes via sentinel-js instead of MutationObserver #18

daurnimator · 2017-09-25T13:11:55Z

Closes #15

I'm a little concerned about sentinel-js's globally visible behaviour: it adds a <style> element to the document. I think this wouldn't be acceptable in some contexts e.g. if you're writing a plugin for a page (think an ad or other embedded content)

daurnimator · 2017-09-25T13:24:11Z

| daurnimator, shouldn't we listen to modification of existing tag also ?
| bgiannan: I don't think modifications to existing JS <script> tags do anything?
| don't know
| bgiannan: did you mean like this? http://jsbin.com/pebinobace/1/edit?html,console
| yes that or a modification of the src attribute
| bgiannan: tested locally modifying <script> src attribute. it always runs what it first loaded as

<html>
<head>
  <title>TEST</title>
  <meta charset="utf8">
</head>
<body>
  <script id="A" type="text/javascript" src="testA.js" async></script>
  <script type="text/javascript">
    var el = document.getElementById("A");
    el.src = 'testB.js';
  </script>
</body>
</html>

amorey · 2017-09-25T13:51:13Z

@daurnimator Are you concerned about the browser allowing SentinelJS to create a <style> tag? This behavior is allowed from javascript so it shouldn't be a problem.

In terms of global style rules, SentinelJS scopes its rules and uses random keys to define animation names so there won't be overlap with other rules:
https://github.com/muicss/sentineljs/blob/master/src/sentinel.js#L53-L63

daurnimator · 2017-09-25T14:04:07Z

@daurnimator Are you concerned about the browser allowing SentinelJS to create a <style> tag? This behavior is allowed from javascript so it shouldn't be a problem.

Was more thinking about people (perhaps non-technical) being surprised by a <style> tag appearing when the fengari-web library is in use (it's meant to just be code, not modifying the document/adding elements to the page).
It would also affect anyone that expects document.styleSheets to be laid out in a certain way.
Though I can't exactly pick who I'm worried about here....

SentinelJS scopes its rules and uses random keys to define animation names so there won't be overlap with other rules

Yep; I saw that.

@amorey are you able to tell us much about performance benefits of the sentinel-js approach? We haven't benchmarked yet, but will removing MutationObserver have a big effect? Do you have links to any benchmarks you've done? Or links to info about browser(s) that fall back to a slow path?

amorey · 2017-09-25T14:22:05Z

Previously, SentinelJS added id="sentineljs" to the style tag but I removed it to save some bytes. Would it help to add it back?

The only performance penalties incurred by SentinelJS are normal CSS lookup penalties (from your CSS selector) and one hash key lookup in javascript on animationstart events:
https://github.com/muicss/sentineljs/blob/master/src/sentinel.js#L25-L28

If you want to detect nodes added at an arbitrary depth in the DOM then SentinelJS should be much faster than using a MutationObserver to watch for changes to the document and iterating recursively through new child elements. However, if you just want to watch a particular element in the tree that doesn't change often then a MutationObserver is probably preferable since it will only get triggered when there's a change to that particular part of the tree. Are you having performance problems with MutationObservers?

daurnimator · 2017-09-25T14:32:55Z

Previously, SentinelJS added id="sentineljs" to the style tag but I removed it to save some bytes. Would it help to add it back?

No I don't think so. I'm probably worried about nothing.

If you want to detect nodes added at an arbitrary depth in the DOM then SentinelJS should be much faster than using a MutationObserver to watch for changes to the document and iterating recursively through new child elements. However, if you just want to watch a particular element in the tree that doesn't change often then a MutationObserver is probably preferable since it will only get triggered when there's a change to that particular part of the tree. Are you having performance problems with MutationObservers?

We haven't benchmarked yet. i.e. this is mostly premature optimization.

However to elaborate on our use case: fengari-web allows you to use lua in the browser just like you use javascript. One feature is that if you create a <script> tag with type="application/lua" and insert it into the document, fengari-web will run it. To do so we need to monitor the whole document for insertions of script tags. Currently we are using a MutationObserver, but I started thinking sentinel-js might be a better answer. It would be bad for our users if writing your website in lua decreased the speed of your webpage by X% just because fengari-web uses a MutationObserver under the hood.

amorey · 2017-09-25T14:35:31Z

Cool, that sounds like an interesting application. Are you using script[type="application/fengari-web"] as a CSS selector? Keep me updated on what you find out!

daurnimator · 2017-09-25T14:40:14Z

Are you using script[type="application/fengari-web"] as a CSS selector?

fengari-web/src/fengari-web.js

Lines 159 to 161 in e602beb

    
           /* the query selector here is slightly liberal, 
        
              more checks occur in try_tag */ 
        
           const selector = 'script[type^="application/lua"] script[type^="text/lua"]';

i.e. we allow <script type="application/lua"> and <script type="text/lua">. We also allow/ignore trailing mime params so that this is also allowed: <script type="application/lua;version=5.3">

amorey · 2017-09-25T14:41:24Z

Great! That's a really nice use of SentinelJS. Please keep me updated on your work!

daurnimator · 2018-02-03T02:05:20Z

This (old) blog post seems to suggest that MutationObserver is faster: http://www.mikehenry.name/2013/06/dom-mutationobserver-performance.html however jsperf links are dead so I can't confirm.

Ping @mhenry07

Use CSS keyframes via sentinel-js instead of MutationObserver

73f2d05

daurnimator requested a review from giann September 25, 2017 13:12

giann approved these changes Sep 25, 2017

View reviewed changes

daurnimator added the enhancement label Sep 25, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use CSS keyframes via sentinel-js instead of MutationObserver #18

Use CSS keyframes via sentinel-js instead of MutationObserver #18

daurnimator commented Sep 25, 2017 •

edited

Loading

daurnimator commented Sep 25, 2017 •

edited

Loading

amorey commented Sep 25, 2017

daurnimator commented Sep 25, 2017 •

edited

Loading

amorey commented Sep 25, 2017 •

edited

Loading

daurnimator commented Sep 25, 2017 •

edited

Loading

amorey commented Sep 25, 2017

daurnimator commented Sep 25, 2017 •

edited

Loading

amorey commented Sep 25, 2017

daurnimator commented Feb 3, 2018

Use CSS keyframes via sentinel-js instead of MutationObserver #18

Are you sure you want to change the base?

Use CSS keyframes via sentinel-js instead of MutationObserver #18

Conversation

daurnimator commented Sep 25, 2017 • edited Loading

daurnimator commented Sep 25, 2017 • edited Loading

amorey commented Sep 25, 2017

daurnimator commented Sep 25, 2017 • edited Loading

amorey commented Sep 25, 2017 • edited Loading

daurnimator commented Sep 25, 2017 • edited Loading

amorey commented Sep 25, 2017

daurnimator commented Sep 25, 2017 • edited Loading

amorey commented Sep 25, 2017

daurnimator commented Feb 3, 2018

daurnimator commented Sep 25, 2017 •

edited

Loading

daurnimator commented Sep 25, 2017 •

edited

Loading

daurnimator commented Sep 25, 2017 •

edited

Loading

amorey commented Sep 25, 2017 •

edited

Loading

daurnimator commented Sep 25, 2017 •

edited

Loading

daurnimator commented Sep 25, 2017 •

edited

Loading