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

Fails with HMR #64

Closed
hoschi opened this issue Sep 4, 2017 · 17 comments
Closed

Fails with HMR #64

hoschi opened this issue Sep 4, 2017 · 17 comments
Labels

Comments

@hoschi
Copy link

hoschi commented Sep 4, 2017

It works fine till you change a file and hot reloading kicks in:
2017-09-04-144503_2058x1409_scrot

It seems the context get lost and the injected theme vanished. Versions:

    "@storybook/addons": "3.2.0",
    "@storybook/react": "3.2.5",
    "storybook-addon-material-ui": "0.8.0",
@usulpro usulpro added the bug label Sep 4, 2017
@usulpro
Copy link
Member

usulpro commented Sep 4, 2017

Thanks for reporting it!
I'll check it out!

@MarkusLanger
Copy link

@usulpro Is there any new information?

@usulpro
Copy link
Member

usulpro commented Sep 14, 2017

@hoschi @MarkusLanger @batazor It's strange! I tried to reproduce it on @storybook/react@3.2.5 and on @3.2.9 and HMR works fine for me.

Could you show the steps or maybe a project to reproduce this bug?

@hoschi
Copy link
Author

hoschi commented Sep 14, 2017

@usulpro unfortunately it is a closed source customer project so I can't share the sources. A minimal repo is also pretty difficult to create as our setup is really powerfull (read: complex as hell ;) ). But I can share the configuration stuff, probably you fix some strange spots?
Material UI version is 0.16.7 if this matters.

src/storybook/config/config.js

import { configure, addDecorator } from '@storybook/react';
import appThemeConfig from 'theme/appThemeConfig';
import intlDecorator from 'storybook/decorators/intl';
import { muiTheme } from 'storybook-addon-material-ui';

let gravityThemeConfig = {
    themeName: 'Gravity',
    ...appThemeConfig,
};
addDecorator(muiTheme([gravityThemeConfig]));

addDecorator(intlDecorator({}));
require('assets/styles-global/app.scss');

const req = require.context('../../', true, /.stories.js$/);

function loadStories () {
    req.keys().forEach((filename) => req(filename));
}

configure(loadStories, module);

src/theme/appThemeConfig.js

import * as muiColors from 'material-ui/styles/colors';
import { fade } from 'material-ui/utils/colorManipulator';

export default {
    palette: {
        primary1Color: muiColors.blue700,
        accent1Color: muiColors.red700,
    },
    appBar: {
        color: muiColors.fullWhite,
        textColor: muiColors.darkBlack,
        padding: 0,
    },
    tabs: {
        backgroundColor: muiColors.fullWhite,
        // use real black as basis, so fading matches MD spec
        textColor: fade(muiColors.black, 0.7),
        selectedTextColor: muiColors.black,
    },
};

src/storybook/config/webpack.config.js

const path = require('path');
const base = path.resolve(__dirname, '../../../');
const dir_stylesGlobal = path.resolve(base, 'src/assets/styles-global');
const dir_icons = path.resolve(base, 'src/assets/images/icons');

module.exports = {
    module: {
        rules: [
            {
                test: /\.css$/,
                use: [
                    'style-loader',
                    {
                        loader: 'css-loader',
                        options: {
                            modules: true,
                            importLoaders: 1,
                            localIdentName: '[name]__[local]__[hash:base64:5]',
                        },
                    },
                    'postcss-loader',
                ],
            },
            {
                test: /\.scss$/,
                exclude: dir_stylesGlobal,
                loaders: [
                    'style-loader',
                    {
                        loader: 'css-loader',
                        options: {
                            modules: true,
                            importLoaders: 2,
                            localIdentName: '[name]__[local]__[hash:base64:5]',
                        },
                    },
                    'postcss-loader',
                    'sass-loader',
                ],
            },
            {
                test: new RegExp(dir_stylesGlobal + '/.*\.scss$'),
                loaders: [
                    'style-loader',
                    'css-loader',
                    'postcss-loader',
                    'sass-loader',
                ],
            },
            {
                test: /\.json$/,
                loader: 'json-loader',
                include: base,
            },
            {
                test: /\.txt$/,
                loader: 'raw-loader',
                include: base,
            },
            {
                test: /\.(png|jpg|jpeg|gif|woff|woff2)$/,
                loader: 'url-loader',
                query: {
                    name: '[path][name].[ext]?[hash]',
                    limit: 10000,
                },
                include: base,
            },
            {
                test: /\.svg$/,
                include: dir_icons,
                loader: 'raw-loader',
            },
            {
                test: /\.svg$/,
                exclude: dir_icons,
                loader: 'url-loader',
                query: {
                    name: '[path][name].[ext]?[hash]',
                    limit: 10000,
                },
            },
            {
                test: /\.(eot|ttf|wav|mp3)$/,
                loader: 'file-loader',
                query: {
                    name: '[path][name].[ext]?[hash]',
                },
                include: base,
            },
        ],
    },
    resolve: {
        extensions: ['.js'],
        modules: [
            base,
            'src/',
            'node_modules',
            path.resolve(base, 'node_modules'),
        ],
    },
};

src/storybook/config/preview-head.html

<link href='https://fonts.googleapis.com/css?family=Roboto:400,300,500' rel='stylesheet' type='text/css'>

src/storybook/config/addons.js

import '@storybook/addon-actions/register';
import 'storybook-addon-material-ui';

src/storybook/config/.babelrc

{
    "presets": ["es2015-node6/object-rest", "react"],
    "plugins": [
        "transform-object-rest-spread",
        "syntax-dynamic-import"  
    ],
    "env": {
    }
}

@usulpro
Copy link
Member

usulpro commented Sep 14, 2017

Thanks @hoschi, now I see where is the problem!

Indeed Storybook has an issue with HMR when you put global decorator outside the configure function. The quickest workaround at this moment is to put it inside. Try the follow:

import { configure, addDecorator } from '@storybook/react';
import appThemeConfig from 'theme/appThemeConfig';
import intlDecorator from 'storybook/decorators/intl';
import { muiTheme } from 'storybook-addon-material-ui';

let gravityThemeConfig = {
    themeName: 'Gravity',
    ...appThemeConfig,
};

require('assets/styles-global/app.scss');

const req = require.context('../../', true, /.stories.js$/);

function loadStories () {
    addDecorator(muiTheme([gravityThemeConfig]));
    addDecorator(intlDecorator({})); // <-- possible it worth to move it as well
    req.keys().forEach((filename) => req(filename));
}

configure(loadStories, module);

@hoschi
Copy link
Author

hoschi commented Sep 14, 2017

@usulpro thats it! Thanks 💯 times, manual reloading was annoying and felt like 1990 😀

@hoschi hoschi closed this as completed Sep 14, 2017
@hoschi
Copy link
Author

hoschi commented Sep 14, 2017

@usulpro ups, forgott about storyshots. They are broken now and stop with an error:

> jest compiled/test/storyshots/all.spec.js                                 

 FAIL  compiled/test/storyshots/all.spec.js                                 
  ● Test suite failed to run          

    Global decorators added after loading stories will not be applied       
                                      
      at ClientApi.addDecorator (node_modules/@storybook/react/dist/client/preview/client_api.js:55:15)                                                 
      at loadStories (evalmachine.<anonymous>:37:29)                        
      at ConfigApi.configure (node_modules/@storybook/react/dist/client/preview/config_api.js:93:9)                                                     
      at evalmachine.<anonymous>:42:22
      at runWithRequireContext (node_modules/@storybook/addon-storyshots/dist/require_context.js:102:3)                                                 
      at testStorySnapshots (node_modules/@storybook/addon-storyshots/dist/index.js:100:35)                                                             
      at Object.<anonymous> (compiled/test/storyshots/all.spec.js:92:31)    
      at process._tickCallback (internal/process/next_tick.js:103:7)        

Test Suites: 1 failed, 1 total        
Tests:       0 total                  
Snapshots:   0 total                  
Time:        3.058s, estimated 6s     
Ran all test suites matching "compiled/test/storyshots/all.spec.js".        
  console.info node_modules/@storybook/react/dist/server/babel_config.js:73 
    => Loading custom .babelrc        

The quickest workaround at this moment is to put it inside.

As you say "workaround" is there a better way to fix the underlying problem? It worked before we upgraded from

"@kadira/storybook": "2.35.3",
"storybook-addon-material-ui": "0.7.6",

to

"@storybook/react": "3.2.5",
"storybook-addon-material-ui": "0.8.0",

@hoschi hoschi reopened this Sep 14, 2017
@hoschi
Copy link
Author

hoschi commented Sep 14, 2017

The workaround you posted has also the problem that storybook ui shows the first story when HMR updates the browser. Obviously I tested it with the first story, so I didn't see that before :(

@usulpro
Copy link
Member

usulpro commented Sep 15, 2017

yep, it's known Storybook issues. :(
I'll try to force solving them.
Maybe it's worth to create issues in the Storybook repo

@usulpro
Copy link
Member

usulpro commented Sep 16, 2017

Hey @hoschi!

Looks like HRM issue was solved in @storybook/react@3.2.8!
At least it disappeared in my test repo after I switch to the latest patch.
And you don't need to keep decorator inside the config function anymore!

Could you try it with your project?

@hoschi
Copy link
Author

hoschi commented Sep 18, 2017

I get an NPM error when I try it:

npm ERR! Linux 4.11.9-1-ARCH          
npm ERR! argv "/home/hoschi/.nvm/versions/node/v6.3.1/bin/node" "/home/hoschi/.nvm/versions/node/v6.3.1/bin/npm" "install"                              
npm ERR! node v6.3.1                  
npm ERR! npm  v3.10.10                
npm ERR! path /home/hoschi/repos/gis/frontend/node_modules/storybook-addon-material-ui/node_modules/js-beautify/js/bin/css-beautify.js                  
npm ERR! code ENOENT                  
npm ERR! errno -2                     
npm ERR! syscall chmod                

npm ERR! enoent ENOENT: no such file or directory, chmod '/home/hoschi/repos/gis/frontend/node_modules/storybook-addon-material-ui/node_modules/js-beautify/js/bin/css-beautify.js'           
npm ERR! enoent ENOENT: no such file or directory, chmod '/home/hoschi/repos/gis/frontend/node_modules/storybook-addon-material-ui/node_modules/js-beautify/js/bin/css-beautify.js'           
npm ERR! enoent This is most likely not a problem with npm itself           
npm ERR! enoent and is related to npm not being able to find a file.        
npm ERR! enoent                       

npm ERR! Please include the following file with any support request:        
npm ERR!     /home/hoschi/repos/gis/frontend/npm-debug.log                  

@usulpro
Copy link
Member

usulpro commented Sep 18, 2017

What node/npm do you use?

@hoschi
Copy link
Author

hoschi commented Sep 18, 2017

Nevermind, it is a bug at js-beautify beautifier/js-beautify#1247

@hoschi
Copy link
Author

hoschi commented Sep 18, 2017

I try the storybook update when this mess is fixed and I can again install the packages ;)

@usulpro
Copy link
Member

usulpro commented Sep 18, 2017

Ok! if it not be fixed until tonight I'll lock this package to the previous working ver

@hoschi
Copy link
Author

hoschi commented Sep 19, 2017

I had to update the related packages also, but this looks good 👍 HMR seems to work, I made some short tests.

2017-09-19-104100_428x145_scrot

@hoschi hoschi closed this as completed Sep 19, 2017
@usulpro
Copy link
Member

usulpro commented Sep 19, 2017

Glad to hear it @hoschi! Let me know if there will be any issues

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

No branches or pull requests

3 participants