search

cssinjs / jss

JSS is an authoring tool for CSS which uses JavaScript as a host language.
https://cssinjs.org
MIT License
7.08k stars 400 forks source link

Warning because of dynamic values during SSR #1277

Open SilverDY opened 4 years ago

SilverDY commented 4 years ago

Describe the bug: I'm using my styles like these:

export default (theme) => ({
    root: {
        position: 'relative',
        display: 'inline-flex',
        alignItems: 'center',
        textAlign: 'center',
        background: theme.bgColor,
        fontWeight: ({weight}) => weight, // here is the problem
    },
});

And I'm getting this error on server-side (client is fine): Warning: [JSS] Rule is not linked. Missing sheet option "link: true". If I delete the dynamic value for fontWeight I will not get this error again. The problem not only in the warning itself, but also in the way how my styles will be applyed in production mode (they break styles in the styles where I'm using dynamic value and server sends me wrong styles after page reloading). P.S. I have tried using styles like this const useStyles = createUseStyles(styles, {link: true}); and it's still not working :(

Codesandbox link: I'm sorry, I have a problem with adding codesandbox.io with SSR and node.js console.

SilverDY commented 4 years ago

By the way, how can I get access to the props values like I do it with theme (export default (theme) => ({...}))? I want to get something like this: export default ({ theme, weight }) => ({...})

m4theushw commented 4 years ago

@SilverDY Could you show all the code you are using? Please include also the dependency versions.

By the way, how can I get access to the props values like I do it with theme (export default (theme) => ({...}))? I want to get something like this: export default ({ theme, weight }) => ({...})

You can't. The first and only argument passed to your styles function is the theme. However you can do the opposite: access the theme inside a dynamic value. Check this.

SilverDY commented 4 years ago

@m4theushw "react-jss": "^10.0.4", A bit simplified code:

serverRenderer.js

const serverRenderer = () => (req, res) => {
    const sheets = new SheetsRegistry();
    const generateId = createGenerateId();

    const content = renderToString(
        <JssProvider registry={sheets} generateId={generateId}>
            <ThemeProvider theme={theme}>
                <Router location={req.url} context={{}}>
                        <App />
                </Router>
            </ThemeProvider>
        </JssProvider>
    );

    return res.send(
        '<!doctype html>' +
            renderToString(
                <Html
                    css={{
                        modules: [
                            res.locals.assetPath('bundle.css'),
                            res.locals.assetPath('vendor.css'),
                        ],
                        jss: sheets,
                    }}
                    scripts={[res.locals.assetPath('bundle.js'), res.locals.assetPath('vendor.js')]}
                >
                    {content}
                </Html>
            )
    );
};

client.js

function Main() {
    return (
        <Router history={history}>
            <ThemeProvider theme={theme}>
                <App />
            </ThemeProvider>
        </Router>
    );
}

hydrate(<Main />, document.getElementById('app'), () => {
    const jssStyles = document.getElementById('jss-server-side');
    jssStyles && jssStyles.parentNode.removeChild(jssStyles);
});

Component.js

...
import styles from './styles';

const useStyles = createUseStyles(styles);

const Component= ({ color }) => {
    const theme = useTheme();
    const classes = useStyles({ theme, color });

    return (
        <div className={classes.component} />
    );
};

Component.defaultProps = {
    color: 'primary'
};

Component.propTypes = {
    color: PropTypes.string,
};

styles.css

export default (theme) => ({
    component: {
        position: 'relative',
        backgroundColor: ({ color }) => theme.palette[color],
        width: 200,
        height: 200,
    },
});

I've tried this too

export default {
    component: {
        position: 'relative',
        backgroundColor: ({ color, theme }) => theme.palette[color],
        width: 200,
        height: 200,
    },
};

After this, I get the warning only on my server-side (node console).

m4theushw commented 4 years ago

Sorry, but I couldn't reproduce this warning. Put your project somewhere for download, maybe having all files we can solve the problem.

leedavidcs commented 4 years ago

@kof @m4theushw @SilverDY I'm actually experiencing this issue as well on NextJS.

I've produced a minimal example here.

What I've naively identified, is that the page emits these errors when there are multiple components on 1 page that renders dynamic styles.

Even when I supplied { link: true } as a second parameter to createUseStyles, the warnings are emitted just the same.

You can download my repo, install dependencies, and run npm run dev to try it out yourselves.

SilverDY commented 4 years ago

@leedavidcs mate, thank you a lot for the example. I was a bit busy to make my own.

MatthD commented 4 years ago

I have the same king of error, same version, crashing my app, I do not use ssr (but use Shadow DOM inside content_script of a web extension), but if I disable my dynamic value it's ok.🤕 Try the link option too, did nothing

alexanderoskin commented 4 years ago

The same issue. For me it happens only with ssr. And one interesting behaviour. When you visit the root page of the website everything is working as excepted. When your start point is any nested page of the website 'Missing sheet option "link: true"' errors thrown for each dynamic rule in styles and all styles are broken.

BEGEMOT9I commented 4 years ago

@kof Hi. Isn’t it necessary to check here the renderer before throwing a warning? https://github.com/cssinjs/jss/blob/fd459d7abf8a8613b7637f14166e83c2b2ef78e3/packages/jss/src/plugins/styleRule.js#L78

kof commented 4 years ago

@BEGEMOT9I yes, using this API when sheet is not attached is basically setting the prop in js only, there is no problem that we need to warn about at this point, but there is one if this is used and link: true is not defined.

BEGEMOT9I commented 4 years ago

@kof Okay, but this warning is logged on the server due to the lack of a renderer. Is there just a way to fix this?

kof commented 4 years ago

@BEGEMOT9I In that case this.renderer is probably null or undefined, so this can be checked, I am happy accept a PR if it contains a test

agorovyi commented 4 years ago

Running into the same issue in Next.JS. I could see the same warning when two or more instances of a component that was created with dynamic props appear on one page. Functionally everything looks and works good. Just wondering if anyone had a chance to add this.renderer check to this warning?

henrykaasik commented 4 years ago

Same issue when i try to use dynamic values. Is there some possibility to fix this issue? Or currently only way is not to use JSS with Next.js?

dipeshhkc commented 4 years ago

@SilverDY @BEGEMOT9I @leedavidcs Did you find any workaround or solutions to turn off this warning on nextjs (SSR) projects?

SilverDY commented 4 years ago

@SilverDY @BEGEMOT9I @leedavidcs Did you find any workaround or solutions to turn off this warning on nextjs (SSR) projects?

Unfortunately, I have to use JSS without these features for now.

leedavidcs commented 4 years ago

@dipesh429 Short answer: No

Long answer: From what I've seen, the warnings seem not to impact styling, but I've decided to try and avoid using dynamic styles as much as possible regardless to be safe.

Just toggling between different classes using the clsx module and using the style prop works well enough in most spots where dynamic styles are desired. If you are on nextjs and are using this strategy, either using the built-in sass-module support or Linaria might be a better route for you.

exlame commented 3 years ago

Same problem with react-jss 10.4 to 10.5.1. Downgrading to 10.0.4 resolves the problem.

SilverDY commented 3 years ago

Same problem with react-jss 10.4 to 10.5.1. Downgrading to 10.0.4 resolves the problem.

It resolves only the problem with this warning (you just don't see it anymore), but the problem with rendering still exists. At least in my case when I have tried this solution.

capi1O commented 3 years ago

Same problem with react-jss 10.4 to 10.5.1. Downgrading to 10.0.4 resolves the problem.

I still have the warning after downgrading to 10.0.4

jacobsfletch commented 3 years ago

Same as described above in my NextJS app, but I've found CSS variables to be a suitable workaround in many cases:

First, define (or redefine) your variable: document.documentElement.style.setProperty('--fontWeight', '500');

Then change your stylesheet from this: fontWeight: ({ weight }) => weight

To this: fontWeight: 'var(--fontWeight)'

CSS variables are subject to the cascade and inherit their value from their parent (aka theming).

sltclss commented 3 years ago

We're migrating our NextJS project from react-jss@8.6.1 to react-jss@10.6.0 and are also seeing no impact to styling but are getting the warnings on SSR. Is there a way to silence this warning? We'd like to continue using the dynamic values feature.

capi1O commented 3 years ago

Here is a patch to silence those warnings on server for react-jss 10.5.0

diff --git a/node_modules/react-jss/node_modules/jss/dist/jss.bundle.js b/node_modules/react-jss/node_modules/jss/dist/jss.bundle.js
index 6ad77dc..80a4a50 100644
--- a/node_modules/react-jss/node_modules/jss/dist/jss.bundle.js
+++ b/node_modules/react-jss/node_modules/jss/dist/jss.bundle.js
@@ -283,7 +283,7 @@ function () {

     var sheet = this.options.sheet;

-    if (sheet && sheet.attached) {
+    if (typeof window !== 'undefined' && sheet && sheet.attached) {
        warning(false, '[JSS] Rule is not linked. Missing sheet option "link: true".') ;
     }

diff --git a/node_modules/react-jss/node_modules/jss/dist/jss.cjs.js b/node_modules/react-jss/node_modules/jss/dist/jss.cjs.js
index af2f66d..c2b8dc4 100644
--- a/node_modules/react-jss/node_modules/jss/dist/jss.cjs.js
+++ b/node_modules/react-jss/node_modules/jss/dist/jss.cjs.js
@@ -235,7 +235,7 @@ function () {

     var sheet = this.options.sheet;

-    if (sheet && sheet.attached) {
+    if (typeof window !== 'undefined' && sheet && sheet.attached) {
       process.env.NODE_ENV !== "production" ? warning__default['default'](false, '[JSS] Rule is not linked. Missing sheet option "link: true".') : void 0;
     }

diff --git a/node_modules/react-jss/node_modules/jss/dist/jss.esm.js b/node_modules/react-jss/node_modules/jss/dist/jss.esm.js
index b77ea3c..16e1ab9 100644
--- a/node_modules/react-jss/node_modules/jss/dist/jss.esm.js
+++ b/node_modules/react-jss/node_modules/jss/dist/jss.esm.js
@@ -221,7 +221,7 @@ function () {

     var sheet = this.options.sheet;

-    if (sheet && sheet.attached) {
+    if (typeof window !== 'undefined' && sheet && sheet.attached) {
       process.env.NODE_ENV !== "production" ? warning(false, '[JSS] Rule is not linked. Missing sheet option "link: true".') : void 0;
     }

diff --git a/node_modules/react-jss/node_modules/jss/dist/jss.js b/node_modules/react-jss/node_modules/jss/dist/jss.js
index 6d3684e..c23e80f 100644
--- a/node_modules/react-jss/node_modules/jss/dist/jss.js
+++ b/node_modules/react-jss/node_modules/jss/dist/jss.js
@@ -289,7 +289,7 @@

       var sheet = this.options.sheet;

-      if (sheet && sheet.attached) {
+      if (typeof window !== 'undefined' && sheet && sheet.attached) {
          warning(false, '[JSS] Rule is not linked. Missing sheet option "link: true".') ;
       }

name it react-jss++jss+10.5.0.patch in folder patches and use https://github.com/ds300/patch-package

hplar commented 3 years ago

Hello everyone,

Looks like this issue is still here and no longer mutable with the above patch by @didrip.

Looking at react-jss, it no longer has a jss dir in node_modules -> only emotion.

Sadly I don't know how to fix this, but would be very grateful if anybody could help out.