Media query styles applied only to the first element in function

[karpcs]

Expected behavior: when changing screen size media query in function should apply styles to all the component that uses it

Describe the bug: only the first component gets the styles of media query in function, it looks like it was introduced in 10.0.1

Codesandbox link: https://codesandbox.io/s/react-jss-playground-nwurq

Versions (please complete the following information):

• jss:10.1.1
• Browser saw it on chrome, havent tested others
• OS windows10:

[pranshuchittora]

@kof if this issue is still open then I would like to work on this :)

[kof]

@pranshuchittora sure

[pranshuchittora]

Thanks @kof for the blazing fast reply :+1:

[pranshuchittora]

-  wrapper: () => ({
    padding: 40,
    background: theme.background,
    textAlign: "left",
    "@media (min-width: 1024px)": {
      backgroundColor: "red"
    }
-  }),
+ wrapper: {
    padding: 40,
    background: theme.background,
    textAlign: "left",
    "@media (min-width: 1024px)": {
      backgroundColor: "red"
    },
+  }, 

@karpcs Converting the style object from an arrow function to plain object works.

@kof probably this is happening due to improper object referencing in arrow function approach. Is it still a bug, if no then I suggest closing this issue :)

[kof]

Feel free to dig deeper and figure out what the actual bug is. Yes it is only when rule function is used.

[kof]

Same as https://github.com/cssinjs/jss/issues/1327

[pranshuchittora]

Rule function is not transpiling ?

[kof]

Might be just the playground, also it's not on the latest version, feel free to update and check what it does.

[pranshuchittora]

I have updated the deps.

    "jss": "^10.1.1",
    "jss-preset-default": "^10.1.1"

Still not working, will take a look at the rule function parsing. Is this the correct function to look at ?

[pranshuchittora]

Hi @kof , Did some preliminary research and found out that the function is not being executed to get the returned object as shown below.

As the function is not bein executed, therefore converting it toCss string returns null.

[karpcs]

I kept the version in playground at the version, that this bug appears, prior versions does not have this bug.

i noticed this when looping through array and passing different props to each and only the first element got the styles, so i just put a little demo here

[kof]

@pranshuchittora yes, you are still on the surface, sheet.update is supposed to call function rules, jss-plugin-rule-value-function plugin is responsible for implementing the function rule API ... somewhere between jss core and the plugin there is a bug, most likely in the plugin.

[kof]

https://github.com/cssinjs/repl/blob/gh-pages/src/index.js#L55 repl doesn't call .update, so on the repl function rules won't work, we need to start calling .update there.

The actual issue in your production code might be in react-jss

[pranshuchittora]

@pranshuchittora yes, you are still on the surface, sheet.update is supposed to call function rules, jss-plugin-rule-value-function plugin is responsible for implementing the function rule API ... somewhere between jss core and the plugin there is a bug, most likely in the plugin.

Yeah, after call update() it worked. I have raised a PR regarding the following fix https://github.com/cssinjs/repl/pull/5

[pranshuchittora]

Till v10.0.0 it was working perfectly. In v10.0.1 the issue of dynamicStyles is occurring.

The line -> https://github.com/cssinjs/jss/blob/d9cca31ad2d058aa0108fa6ec48fe872b0e7e886/packages/jss/src/DomRenderer.js#L406

Is trying to concert the dynamic style to string, which is unable to.

This flow is encountered when the link is true. IMO the issue is in jss not react-jss.

P.S. update method on rule is not available.

[kof]

That line was changed last time before 10.0.0

[pranshuchittora]

Hi @kof , So I ran git bisect between the v10.0.0 & v10.0.1. And found that the commit which introduced that bug was 9f7924eb79bf1e78816db7aaa4cf324471.

Reverting the changes in packages/react-jss/src/createUseStyles.js introduced in the following commit fixes the bug.

[kof]

Can you create a test? And optionally a fix?

On Sat, May 9, 2020, 11:31 Pranshu Chittora [email protected] wrote:

Hi @kof https://github.com/kof , So I ran git bisect between the v10.0.0 & v10.0.1. And found that the commit which introduced that bug was 9f7924eb79bf1e78816db7aaa4cf324471 https://github.com/cssinjs/jss/commit/9f7924eb79bf1e78816db7aaa4cf324471 .

[image: Screenshot from 2020-05-09 14-25-42] https://user-images.githubusercontent.com/32242596/81469474-a0960e80-9202-11ea-8a79-b4c0983f3d28.png

Reverting the changes in packages/react-jss/src/createUseStyles.js introduced in the following commit fixes the bug.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/cssinjs/jss/issues/1320#issuecomment-626136881, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAM4WAFNYSXDJPUS5IZHLDRQUPGFANCNFSM4LTOR7UA .

[pranshuchittora]

@kof kindly check #1343 for the fixes :)

[jmikrut]

This is still an issue, correct? I am on 10.6.0 and can reproduce.

[kof]

https://github.com/cssinjs/jss/pull/1343#issuecomment-808819503

[mjeanroy]

Hi @Kof,

I tried to debug this, and I would like to share with you what I found. With my coworker @pybuche, we tried to understand what was the issue here, and he found something interesting: if we wrap the call to manageSheet in a setTimeout, the issue disappears! We tried to understand why, and here is my explanation:

1. When the first component renders, everything works fine. In fact, the sheet is correct, and all the rules are deployed using manageSheet, the sheet is then marked as attached.

2. When the second component renders, the sheet is already attached, but all its dynamic rule are updated.

If we wrap manageSheet in a setTimeout, then, when the second component renders, the sheet is not attached yet to the DOM, so it works because all the rules are attached & deployed properly.

The issue really occurs when updating a dynamic rule, in fact, if we do:

if (newSheet) {
  // Detach the sheet so all rules will be inserted
  unmanageSheet({
    index,
    context,
    sheet: newSheet,
    theme
  });

  manageSheet({
    index,
    context,
    sheet: newSheet,
    theme
  });
}

=> It also fixes the issue.

I tried to go deeper to understand why updating dynamic rules did not works as exepected, and I think the issue is in jss-plugin-nested, especially here:

} else if (isNestedConditional) {
  // Place conditional right after the parent rule to ensure right ordering.
  container
    .addRule(prop, {}, options)
    // Flow expects more options but they aren't required
    // And flow doesn't know this will always be a StyleRule which has the addRule method
    // $FlowFixMe[incompatible-use]
    // $FlowFixMe[prop-missing]
    .addRule(styleRule.key, style[prop], {selector: styleRule.selector})
}

The line container.addRule insert rule, but in fact, the rule is not yet up to date, since the child rule is added the line after (to summarize, insertRule happens too early I think).

I tried to fix it by splitting the rule addition and the insertion, something like this:

const addedRule = container.createRule(prop, {}, options);
addedRule.addRule(styleRule.key, style[prop], {selector: styleRule.selector})
container.deployRule(addedRule);

You can find the commit on my fork here. Note that it is just a POC, I did not try to make it "super clean", but if you think the solution seems OK, feel free to give your opinion and I can work on a PR :)

In the meantime, I would like to suggest a workaround until the issue is fixed: adding a plugin that detach and re-attach the sheet, such as:

jss.use({
  onUpdate(data: any, rule, sheet, options) {
    if (sheet) {
      sheet.detach().attach();
    }
  },
});

This workaround can definitely be improved, that is just the basic idea :)

Also, thanks for your amazing library, that's an impressive work, and I really love using it

[EDIT] I opened #1523 to get your feedback

[hkar19]

definitely @mjeanroy suggestion to use:

jss.use({
  onUpdate(data: any, rule, sheet, options) {
    if (sheet) {
      sheet.detach().attach();
    }
  },
});

is very nice.

I just want to add, if you are using react-jss, remember that using jss.use means you need to do your custom setup.

therefore, you need to do things manually such as:

1. dont forget to call preset from jss-preset-default in your createJss, such as:

import { create as createJss, Rule, StyleSheet } from "jss";
import { JssProvider, ThemeProvider } from "react-jss";
import preset from "jss-preset-default";

// i made my array of plugins here, and spread it in my jss.use
import jssPlugins from "./jssPlugins";

// dont forget this!
const jss = createJss(preset());
jss.use(
  {
    onUpdate(data: object, rule: Rule, sheet: StyleSheet) {
      if (sheet) {
        sheet.detach().attach();
      }
    },
  },
  ...jssPlugins
);

ReactDOM.render(
  <React.StrictMode>
    <JssProvider jss={jss}>
      <ThemeProvider theme={theme}>
        <App />
      </ThemeProvider>
    </JssProvider>
  </React.StrictMode>,
  document.getElementById("root")
);

2. in jssPlugins.ts, i do:

import a from "jss-plugin-camel-case";
import b from "jss-plugin-compose";
import c from "jss-plugin-default-unit";
import d from "jss-plugin-expand";
import e from "jss-plugin-extend";
import f from "jss-plugin-global";
import g from "jss-plugin-nested";
import h from "jss-plugin-props-sort";
import i from "jss-plugin-rule-value-function";
import j from "jss-plugin-rule-value-observable";
import k from "jss-plugin-template";
import l from "jss-plugin-vendor-prefixer";
// add any more plugins you desire

export default [a(), b(), c(), d(), e(), f(), g(), h(), i(), j(), k(), l()];

this workaround is tedious, but for now this is all we got

---

UPDATE: actually this method will give some annoyance in storybook where some styles got lost if you are using style as function. if you're using style as object, it would be working fine.

for example: this will be fine

content: {
      flex: 1,
      overflowY: "scroll",
      // overflowX: ""
      maxHeight: "100vh",
      backgroundColor: theme.defaultBackground,
    }

some of these style will be lost somehow, (only in storybook, btw).

content: ()=>({
      flex: 1,
      overflowY: "scroll",
      // overflowX: ""
      maxHeight: "100vh",
      backgroundColor: theme.defaultBackground,
    })

[j4mesjung]

Hello, any updates on a fix for this? Experiencing this issue on 10.9.0

[TchernyavskyDaniil]

Sorry if I offended or don't understand your processes, I'm speaking as a regular consumer of your library

This bug is from version 10.0.1. Shouldn't it be fixed by the current library team? Part of my team is giving up on jss, as well as some people I know because of this problem. I honestly don't quite understand the point of preparing a move to 18 React if there are such critical bugs

[kof]

I am merging good PRs that make progress, if a PR ads support for react 18 and has tests etc - cool.

if you want to get something fixed, please fix it, add a test - I will merge it too.