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

Css loading complexity in angular app #2688

Closed
igor-dv opened this issue Jan 8, 2018 · 8 comments
Closed

Css loading complexity in angular app #2688

igor-dv opened this issue Jan 8, 2018 · 8 comments

Comments

@igor-dv
Copy link
Member

igor-dv commented Jan 8, 2018

I would like to discuss here the problem we have with the css loading complexity for the angular app.

As I understand from the recent issues, there are several flows of loading css to the app
(Related issues: #2647, #2629, #2616)

Global Import

Example:

import './some.css'
...
storyOf(...)
  .add(...);

or even in the config.js

import '../bootstrap.css'
...
configure(loadStories, module);

Storybook should add it to the style tag in iframe

With the "styleUrls"

Example:

@Component({
  selector: 'storybook-app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.css'], // <---
})
export class AppComponent {
  title = 'app';
}

In this case, it should be loaded via raw-loader, and angular will add it as it wishes:

The problem

Our angular app includes two rules for css:

1, in webpack.config.js:

{
  test: /\.(html|css)$/,
  loader: 'raw-loader',
  exclude: /\.async\.(html|css)$/,
}

2, in defaults/webpack.config,js:

{
  test: /\.css$/,
  use: [
    require.resolve('style-loader'),
    loader: require.resolve('css-loader'),
    loader: require.resolve('postcss-loader'),
  ]
}

And we are getting some collision between these rules, so people need to extend webpack.config.js in order to get their angular components work in Storybook (Example )

IMO extending webpack.config.js should be for some specific/rare usecases, but not for the common usage.

@storybooks/team WDYT?

@ndelangen
Copy link
Member

IMO extending webpack.config.js should be for some specific/rare usecases, but not for the common usage.

Agree 💯

We have absolutely no preference in the webpack config ourselves in regards to css, we add things so most users have maximum convenience.

@Quramy
Copy link
Contributor

Quramy commented Jan 10, 2018

@igor-dv I'm very interested in this issue.

people need to extend webpack.config.js

The webpack configuration created by Angular CLI is too complex. So it's not easy to extend @storybook/angular/server.

I think Storybook should provide the way to configure webpack Angular CLI based project.

And I've created the helper function for PoC:
https://github.com/Quramy/storybook-angular-cli-helper#install

@igor-dv
Copy link
Member Author

igor-dv commented Jan 10, 2018

@Quramy , looks promising ! Don't you think it's better to make it as a default behavior? So nobody will need to extend webpack.config?

@Quramy
Copy link
Contributor

Quramy commented Jan 10, 2018

@igor-dev Absolutely yes, I do 😄

I hope the following behavior:

  • If user project has .angular-cli.json, Storybook serves with configuration Angular CLI specific(CSS/SASS rules, global styles and so on)
  • Otherwise, serves with the minimum configuration

@alterx
Copy link
Member

alterx commented Jan 11, 2018

@Quramy @igor-dv +1 to this :D
What's missing here to have something we feel comfortable to ship?

This was referenced Jan 11, 2018
@Quramy
Copy link
Contributor

Quramy commented Jan 12, 2018

@alterx Thanks for your feedback. I'll create this as a PR in this weekend !

@alterx
Copy link
Member

alterx commented Jan 12, 2018

Perfect @Quramy, let us know if we can help

@alterx
Copy link
Member

alterx commented Jan 19, 2018

Merged and released, fix available in https://github.com/storybooks/storybook/releases/tag/v3.4.0-alpha.5

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

No branches or pull requests

4 participants