johanholmerin / style9

CSS-in-JS compiler inspired by Meta's stylex
MIT License
570 stars 27 forks source link

Next.js hot-reload not working for new styles #47

Closed TxHawks closed 3 years ago

TxHawks commented 3 years ago

Using the withStyle9 plugin, hot-reload only sort of works: When authoring styles, classes will correctly be attached to elements on hot-reload, but a new css file isn't generated, so if the class did not previously exist already, no style is associated with the attached class. A hard reload fixes this.

So, for example, if we have:

const styles = style9.create({
  foo: {
    color: '#333',
    display: 'flex',
   paddingLeft: '2em',
   paddingRight: '2em',
  },
   bar: {
     marginTop: '2em',
   },
});

And we than add paddingLeft: 2em to bar, that will work fine, because there is already a class associated with paddingLeft: 2em in the css file. However, if we were to add, for instance, color: 'green', to either foo or bar, a new class would be attached to the relevant element(s), but it would have no effect, because there is no class associated with that style already in the css file, and no new css file is being generated.

johanholmerin commented 3 years ago

Can confirm that Next.js does not load new CSS files when reloading components using Fast Refresh. Could you try changing isDevelopment to options.dev(or true) in next.js? I disabled it because it was troublesome but it should fix this, in which case we would need to enable it again.

TxHawks commented 3 years ago

Changing isDevelopment to true or options.dev fixes the issue

johanholmerin commented 3 years ago

The reason I have disabled that option is that it uses style-loader, which loads each file individually and inserts it into style tags. This causes issues when using media queries and pseudo selectors, because they need to be sorted together make sure they get deterministically applied. Without sorting the applied styles may change depending the last edited file.

The possible solutions are:

  1. Make MiniCssExtractPlugin support HMR. It already should, not sure why it doesn't in Next.js
  2. See if there is a way to combine and sort the CSS files when using style-loader. Might mean to invasive changes in the Next.js Webpack config.
  3. Sort CSS files on the client in development. Would require loading PostCSS on the client, might be slow but should be possible.
  4. Use repeated selectors instead of sorting for specificity, replacing .a:hover with .a.a:hover. Would like to avoid this since it's a major change to style9 just to fix a Next.js issue.
  5. Disable HMR/require full page reload
johanholmerin commented 3 years ago

@TxHawks Can you confirm that changing the following lines in next.js fixes MiniCssExtractPlugin HMR, possibly only for Next.js 10.2.1 or higher?

-filename: 'static/css/[contenthash].css',
-chunkFilename: 'static/css/[contenthash].css',
+filename: 'static/css/[name].css',
+chunkFilename: 'static/css/[name].css',

It's also available in the branch next-hmr, which should fix it for all versions.

TxHawks commented 3 years ago

Sorry for the delay in responding, was away from a computer.

Can confirm that it does indeed work. Thank you!

johanholmerin commented 3 years ago

Thanks for the help. Fixed in 0.10.10