Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

covering more cases #112

Closed
trusktr opened this issue Sep 16, 2020 · 10 comments
Closed

covering more cases #112

trusktr opened this issue Sep 16, 2020 · 10 comments

Comments

@trusktr
Copy link

trusktr commented Sep 16, 2020

The README didn't mention when stylsheet content updates, or other similar event, as being detected to trigger resize observations.

I suppose that can get complicated.

@TremayneChrist
Copy link
Member

Hi @trusktr can you expand on this a bit?

  • Do you mean when <style/> tag content changes it triggers an update? Or,
  • Do you mean when an external stylesheet changes, it does not update?

@trusktr
Copy link
Author

trusktr commented Sep 16, 2020

Both, but actually there's a few ways to trigger style changes. F.e.:

  • Changes to <style>'s DOM (which results in changing the CSS OM). MutationObserver can detect this.
  • Changes to CSS OM in document.styleSheets (or any <style> or <link> element's .sheet property). These changes do not change DOM content, so are not detectable by MutationObserver. For example, try running the following in devtools while here on GitHub:
    document.querySelector('link[rel=stylesheet]').sheet.addRule('body', 'background: red')
  • new CSSStyleSheets can be added to document.adoptedStyleSheets
  • new CSSStyleSheets can be added to the adoptedStyleSheets of any shadow root

Top level MutationObserver can't observe anything in ShadowRoots. We'd need to monkey-patch attachShadow to make that work, so that we can add a MutationObserver per each ShadowRoot.

I've been thinking about similar things over at lume/lume#159. I'll eventually get to it.

@TremayneChrist
Copy link
Member

So to improve the readme - changes could be made to say what the polyfill does not detect. Then it is safe to assume any other modifications which could alter an elements size, will trigger a notification.

Would this help to fix the readme issue?

@TremayneChrist
Copy link
Member

Shadow DOM support will come (#42) and I think it's possible without a monkey patch 🤞

@trusktr
Copy link
Author

trusktr commented Sep 17, 2020

Curious if it is possible without patching attachShadow. Is there another way to get any document when it is created?

@trusktr
Copy link
Author

trusktr commented Sep 17, 2020

Would this help to fix the readme issue?

Yeah probably the list in Limitations would be updated.

@TremayneChrist
Copy link
Member

Curious if it is possible without patching attachShadow. Is there another way to get any document when it is created?

Well I was hoping not to, however, that may not be the case. It depends on how many use cases to cover with shadow root vs performance of adding all those listeners and a monkey patch. Plus the patch has to be loaded before any shadow roots are created...

@trusktr
Copy link
Author

trusktr commented Sep 17, 2020

Plus the patch has to be loaded before any shadow roots are created...

I think that's fine; as with any polyfill it needs to be loaded first before code uses it. Native ResizeObserver is available before any user code ever runs, so I think it would be fair to tell users to load the polyfill before everything else too.

As far as I know, I think this is the only way to hook onto the shadow roots:

{
  const original = Element.prototype.attachShadow
  Element.prototype.attachShadow = function(...args) {
    const root = original.apply(this, args)
    new MutationObserver(...).observe(root)
    return root
  }
}

I wonder how to intercept CSS OM APIs. If the engine calls the APIs (like addRule) whenever it is creating/modifying the CSS OM based on DOM changes (f.e. parsing <style> during HTML parse, or later changes to <style> DOM), then that would should work. But I know in some cases the engine bypasses the JS APIs internally (f.e. it does not call setAttribute in order to set DOM attributes by any non-user means like parsing).

EDIT: Hmm, now that I think about it, we can detect initial styling with MutationObserver, then after that MutationObserver keeps triggering for DOM updates, and a monkey-patched CSS OM will detect user changes. So MutationObserver has the non-user cases covered (which I think are only parsing).

Here's an example that shows that the browser engine bypasses the user-exposed APIs when creating the CSS OM. Try the following in Chrome console to see there will be no output:

{
  const original = CSSStyleSheet.prototype.addRule 
  CSSStyleSheet.prototype.addRule = function(...args) {
    console.log('add rule!')
    return original.apply(this, args)
  }
}

{
  const original = CSSStyleSheet.prototype.replaceSync 
  CSSStyleSheet.prototype.replaceSync = function(...args) {
    console.log('replace sync!')
    return original.apply(this, args)
  }
}

{
  const original = CSSStyleSheet.prototype.replace
  CSSStyleSheet.prototype.replace = function(...args) {
    console.log('replace!')
    return original.apply(this, args)
  }
}

{
  const original = CSSStyleSheet
  window.CSSStyleSheet = class extends original {
    constructor(...args) {
      console.log('CSSStyleSheet constructor!')
      super(...args)
    }
  }
}

// I was hoping this would result in some output, but the engine creates the CSS OM somehow else,
// or maybe it holds references to the original non-patched APIs.
document.body.insertAdjacentHTML('beforeend', `
  <style>
    body { background: lightblue; }
  </style>
`)

But at least we know, if the user calls replace{Sync} or addRule we can trigger an observation in that case. We'd need to track which documents the stylesheet is applied in.

@TremayneChrist
Copy link
Member

Initial Shadow DOM support has been add to the v4.0.0-1 release. Hopefully this helps with some things.

I think the only thing left, which seems impossible to support without a monkey-patch, is the in-memory stylesheet changes and adopted stylesheets.

@TremayneChrist
Copy link
Member

Added more info in the v4 readme. Will clean up the docs later, but for now, I think it's ok.

@trusktr feel free to open a new issue regarding CSSOM and constructing/adopting stylesheets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants