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

cache option 'prepend' doesn't work when use '@emoton/react' 's Global Component #2790

Open
tobemaster56 opened this issue Jun 22, 2022 · 9 comments

Comments

@tobemaster56
Copy link

Current behavior:
For more complex reasons, both emotion and react-jss exist in my project. I want the styles provided by react-jss to more easily override the styles provided by the '@emotion/react' Global component. So I expect Global to insert the style front in the head tag.

I see that there is a prepend option in the cache, but the styles generated with Global don't work.

image

To reproduce:
https://codesandbox.io/s/jolly-resonance-6mrxim?file=/src/App.js

Expected behavior:

cache option 'prepend' works for Global Component

Environment information:

  • react version: 18.0.0
  • @emotion/react version: 11.9.3
@tobemaster56
Copy link
Author

i found the key code here

useInsertionEffect(function () {
    var key = cache.key + "-global"; // use case of https://github.com/emotion-js/emotion/issues/2675
    
   // the reason is here,ignore cache.sheet.prepend,Is it intentional?
    var sheet = new cache.sheet.constructor({
      key: key,
      nonce: cache.sheet.nonce,
      container: cache.sheet.container,
      speedy: cache.sheet.isSpeedy,
    });
    
   // ......

    sheetRef.current = [sheet, rehydrating];
    return function () {
      sheet.flush();
    };
  }, [cache]);

image

@tobemaster56
Copy link
Author

tobemaster56 commented Jun 22, 2022

in this issue: #2774, i got the follows

cache option prepend is deprecated, use insertionPoint instead,but the problem still exists。

// cache.sheet.insertionPoint was ignored,Is it intentional?
new cache.sheet.constructor({
      key: key,
      nonce: cache.sheet.nonce,
      container: cache.sheet.container,
      speedy: cache.sheet.isSpeedy,
    });

@Andarist
Copy link
Member

I'm actually not sure - perhaps we should do some digging in the issues, the original PR etc to see if there was any specific reason to not use this option here. I can think of some edge cases around that if the global style gets inserted before a scoped one but that should be a somewhat rare situation.

@tobemaster56
Copy link
Author

@Andarist thank you for your reply.

It took me quite some time to find out why the style insertion of the Global component is not controlled by the cache option prepend or insertionPoint . Even if the global style needs to be inserted before the scoped style, it is a rare case. I hope to give the user the choice.

In my opinion, the global style represents only the global, not restricted by a scope, and should not be restricted to be inserted before or after.

Translated with www.DeepL.com/Translator (free version)

@tobemaster56
Copy link
Author

Finally, I did some hacking to ensure that the insertion behavior of the global style is controlled by Cache's options

import createCache, { Options as CacheOptions } from '@emotion/cache';
import { StyleSheet, Options } from '@emotion/sheet';

export const createEmotionCache = (options: CacheOptions) => {
  const cache = createCache(options);

  /**
   * A custom styleSheet is used here because emotion's Global component creates a separate styleSheet,
   * making some of the cache's options ineffective, like 'prepend' and 'insertionPoint',
   * details: https://github.com/emotion-js/emotion/issues/2790
   */
  class MyStyleSheet extends StyleSheet {
    constructor(options?: Options) {
      super(options);
      // hack emotion's Global new styleSheet
      // @ts-ignore
      this.prepend = cache.sheet.prepend;
      // @ts-ignore
      this.insertionPoint = cache.sheet.insertionPoint;
    }
  }

  const isBrowser = typeof document !== 'undefined';
  let container: Node;
  if (isBrowser) {
    container = options.container || document.head;
  }
  // @ts-ignore
  cache.sheet = new MyStyleSheet({
    key: options.key,
    // @ts-ignore
    container: container,
    nonce: options.nonce,
    speedy: options.speedy,
    prepend: options.prepend,
    insertionPoint: options.insertionPoint,
  });

  return cache;
};

@igorovic
Copy link

igorovic commented Aug 27, 2022

I have a similar issue with the option {prepend: false} but it happens only on production build.
I tried the insertionPoint method but it has the same behavior.

reproduction repo: https://github.com/igorovic/mantine-prepend-issue

client side

ssr result

@Andarist
Copy link
Member

@igorovic perhaps you could figure out how to fix this based on this. IIRC the issue was awfully similar there and I've managed to make it work.

@igorovic
Copy link

Hi,

I managed to make it work after many trials. Here is an example repo if it could help anyone facing the same issue.

https://github.com/igorovic/mantine-prepend-issue

@siriwatknp
Copy link

siriwatknp commented Sep 9, 2024

I'm actually not sure - perhaps we should do some digging in the issues, the original PR etc to see if there was any specific reason to not use this option here. I can think of some edge cases around that if the global style gets inserted before a scoped one but that should be a somewhat rare situation.

This is not an edge case since CSS variable is widely supported. It's quite common that the CSS variables are applied to :root using Global:

<Global styles={{ ":root": { …variables } }} />

Those variables should be overridden by a consumer app with alternative styling solution like plain CSS or SCSS but since prepend: true is not working for Global, the overridden is not possible.

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

No branches or pull requests

4 participants