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

Reuse styles between server and client #1132

Closed
wants to merge 1 commit into from

Conversation

frenzzy
Copy link
Member

@frenzzy frenzzy commented Feb 12, 2017

  • reuse styles between server and client, it should help to solve double animations and svg font disappearance
  • isomorphic-style-loader replaced with custom inline-css-loader which is modified version of original css-loader
  • better hot module replacement support by custom withStyles component

Styles

P.S.: This is an experimental PR and these changes probably won't be merged into the master branch as they are. To be discussed...

P.P.S: Your help with testing is very appreciated. Thanks!

@langpavel
Copy link
Collaborator

@frenzzy Is there a reason why you cannot/don't want publish this as a library or update to isomorphic-style-loader itself?

@frenzzy
Copy link
Member Author

frenzzy commented Feb 17, 2017

I really want to make this stuff available as external module but the desired solution touches different parts of application and build process. Here is my vision of composite parts:

  1. webpack loader (like css-loader)
    • should provide class names if CSS Modules enabled for js code
    • should provide access to cssText for inlining or critical path css
    • should provide stylesheet file identificator (or module id)
    • should be listed inside dev dependencies
  2. webpack plugin (like AssetsWebpackPlugin)
    • should emit a module (or file) with CSS Modules classnames for functional tests
      example:
      {
        "src/components/Header.css": {
          "root": "Header-root-O9oW9",
          "nav": "Header-nav-3pohg"
        },
        "src/components/Navigation.css": {
          "root": "Navigation-root-2gcJx",
          "link": "Navigation-link-Ntl35"
        }
      }
    • should be listed inside dev dependencies
  3. client-side document.head tag controller (like react-helmet)
    • should provide ability to insert meta, link, style or script tags with any attributes into document.head on client-side
    • should have high performance, for example don't touch DOM if no changes required and use lazy initialization
    • should care about cross-browser compatibility
    • should be listed inside dev dependencies because used for client-side only
  4. server-side critical css path collector (like redux Provider)
    • should extract the styles for all the rendered react components and insert them into the <head> before responding
    • should be listed inside dependencies for server
  5. higher order react component (like withStyles decorator)
    • should use 3 on client-side inside componentWillMount/componentWillUnmount react life cycle methods
    • should provide styles for 4 on server-side
    • should support hot module replacement for mounted styles and this probably requires properly configured react-hot-loader v3 with its deepForceUpdate of react components
    • should be listed inside dependencies for both client and server sides

Suggestions about how to move it into module(s) are welcome.

@langpavel
Copy link
Collaborator

It is still in my mind but I'm loosing idea about how this works.
@frenzzy If you believe it will increase performance, decrease bandwidth and is backward compatible then I will test this on my projects.
If you are sure that this works nicely, you have 👍 from me for merge 😈

@woffleloffle
Copy link

Hey @frenzzy,

I've managed to implement this in a modified fork of RSK, but the hot styles are now always 1 version behind.

As an example, if I start out with:

body {
  background: white;
}

...and run npm start - the project spins up fine.
I then change the stylesheet to:

body {
  background: orange;
}

...and save the file.
The HMR logs show up in the browser, but the styles don't update.

I then change the CSS to:

body {
  background: black;
}

...and the CSS hot-updates and the background is now orange.

I dug around the code trying to find the culprit, and I have a feeling it's in your custom loader, or the withStyles HOC componentWillReceiveProps.

I'd really appreciate a pointer.

Thanks!

@frenzzy
Copy link
Member Author

frenzzy commented Sep 26, 2017

@WillemLabu hey, in this PR after hot update received, happens following:

  1. webpack triggers module.hot.accept(...) in src/client.js
  2. we replace old components with new one by require call
  3. then call deepForceUpdate (maybe it does not update some types of components?)
  4. it triggers componentWillReceiveProps in withStyles HOC
  5. it inserts new styles to the DOM (replaces old ones)

@woffleloffle
Copy link

woffleloffle commented Sep 26, 2017

Not too familiar with how the HMR bubbles through the app, but it seems the module.hot.status() on Line 89 of the withStyles.js HOC doesn't propagate properly, and is set to idle on the first file change, and apply on the second, which then repeats.

I've just removed the module.hot.status() check - meaning the following is called on every componentWillReceiveProps:

// Enable Hot Module Replacement (HMR)
if (module.hot) {
  WithStyles.prototype.componentWillReceiveProps = function componentWillReceiveProps() {
    this.inserted = styles.map(s => insertCss(s.getCss(), true));
  };
}

It's probably not the best way of going about things, but it works.

@langpavel
Copy link
Collaborator

langpavel commented Mar 27, 2018

Hi @frenzzy, not sure if directly related, but
I found great solution with Webpack 4,

You can use MiniCssExtractPlugin in production, style-loader in development for client,
and isomorphic-style-loader for server in both modes (only using extracted class names, nothing more…)
Then you can load all CSS chunks in production (as files or embed them in <style data-href="…">)
See https://github.com/webpack-contrib/mini-css-extract-plugin#using-preloaded-or-inlined-css

You don't need withStyles HoC any more! 🎉

What do you think? I'm just working on this for my customer and it looks working really nicely ❤️

@frenzzy
Copy link
Member Author

frenzzy commented Mar 27, 2018

Yes, maybe.. need to try it out.
I was hoping that new css-loader will be released some day and we can drop isomorphic-style-loader at all.

@ulani
Copy link
Member

ulani commented May 27, 2021

@frenzzy thank you very much for this PR! Unfortunately, we have close it due to inactivity. Feel free to re-open it or join our Discord channel for discussion.

NOTE: The main branch has been updated with React Starter Kit v2, using JAM-style architecture.

@ulani ulani closed this May 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants