Issue with the insertion of stylesheet

pionxzh commented 1 year ago

Hi, thanks for creating this awesome library.

I use sentineljs in userscript to monitor elements on the page. But the style injected by sentineljs will be blown away by things like next/head.(ref)

I would like to know why it uses insertBefore to inject the style to the start of head.

https://github.com/muicss/sentineljs/blob/c3ff7d7b1fede386c1558fdacec7601e5547ba6f/src/sentinel.js#L38-L42

By adopting this change can fix the problem I mentioned.

       // add stylesheet to document
       styleEl = doc.createElement('style');
-      head.insertBefore(styleEl, head.firstChild);
+      head.append(styleEl);
       styleSheet = styleEl.sheet;
       cssRules = styleSheet.cssRules;

I understand that asking the library to adapt to this specific usecase might be weird. Please let me know what you think. 🙏

amorey commented 1 year ago

Happy to hear you're finding the library useful! It's been a while but I think .insertBefore() is used so that the library doesn't overwrite any user-defined css but I'd have to take a deeper look to be sure.

It looks like .append() isn't available in IE (https://caniuse.com/mdn-api_element_append) but .appendChild() should be (https://caniuse.com/mdn-api_node_appendchild).

Which mechanism does next/head use that blows away .insertBefore() but not .append()?

pionxzh commented 1 year ago

Ok. I looked into how next/head works. And https://github.com/vercel/next.js/issues/11012#issuecomment-1441983776 explains it well.

There is a meta element looks like <meta name="next-head-count" content="12"/>. <HeadManager /> will iterate over previous elements for every 'meta', 'base', 'link', 'style', 'script' it finds, and remove it. That's why using it on websites powered by next.js is problematic.

I think it's reasonable not to break the priority of rules with insertBefore (). Here are the solutions I can think of:

delay the execution of sentineljs by user, for example like onload.
inject the style before the "first" style/link like what tippy(ref) do. but I'm not 100 sure about this will ultimately fix the issue, I will test it later.
~~patch it like what I have already done~~

amorey commented 1 year ago

Thanks! It's helpful to understand what next/head is doing under the hood. Would using a pre-defined style tag help to get around the issue? For example, we could have sentinel load itself into a style tag with id="sentinel-css" if it exists:

// _document.js
import { Html, Head, Main, NextScript } from 'next/document';

export default function Document() {
  return (
    <Html>
      <Head>
        <style id="sentinel-css" />
        <script src="/path/to/sentinel.js" />
      </Head>
      <body>
        <Main />
        <NextScript />
      </body>
    </Html>
  )
}

pionxzh commented 1 year ago

Good idea. This should work for application owners and people who wrote userscript like me.

amorey commented 1 year ago

Great, I pushed the change to 0.0.7-rc1: https://www.npmjs.com/package/sentinel-js

Let me know if it solves the problem.

pionxzh commented 1 year ago

@amorey I noticed that the #sentinel-css is still being moved to the start of head. Is this expected? or leave it to be controlled by user? So if users want to provide a style element by themself, they need to be responsible for the potential conflict or insert the style to an appropriate position.

styleEl = doc.getElementById("sentinel-css")
if(!styleEl) {
  styleEl  = doc.createElement('style');
  head.insertBefore(styleEl, head.firstChild);
}
styleSheet = styleEl.sheet;
cssRules = styleSheet.cssRules;

pionxzh commented 1 year ago

Btw, in my case, the ChatGPT website no longer blows away the sentinel style. Not sure what changed. But I think this is still a good escape hatch for people who might be affected by similar mechanisms.

amorey commented 1 year ago

Sorry, that was a mistake. Good catch! It's fixed in 0.0.7-rc2. Try it out and let me know if it works.

Btw, in my case, the ChatGPT website no longer blows away the sentinel style. Not sure what changed. But I think this is still a good escape hatch for people who might be affected by similar mechanisms.

Which version is working 0.0.6, 0.0.7-rc1 or both?

pionxzh commented 1 year ago

Which version is working 0.0.6, 0.0.7-rc1 or both?

Nah, I think it was fixed/changed by Next.js or ChatGPT developers. I encountered this issue 6 months ago. And never go back to check it again.

pionxzh commented 1 year ago

0.0.7-rc2 works for me. 👍

amorey commented 1 year ago

Great, thanks for testing it out. The new code is live in 0.0.7.

kubetail-org / sentineljs

Issue with the insertion of stylesheet #20