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

Does not bundle all imported css files #102

Closed
danez opened this issue Apr 17, 2018 · 12 comments
Closed

Does not bundle all imported css files #102

danez opened this issue Apr 17, 2018 · 12 comments

Comments

@danez
Copy link

danez commented Apr 17, 2018

I have a weird problem:
I have a js file that imports a css file and a less file. The imported less file is included in the extracted file the imported css file is not.

Now I think this might have todo because I recently added code splitting here: https://github.com/reactjs/react-tabs/blob/master/examples/src/components/ExampleItem.js#L19 But this async import and the import of the css should not influence each other as far as I understand.

The output of webpack looks like this, which does not really help me:

Child mini-css-extract-plugin ../../node_modules/css-loader/index.js!../../node_modules/less-loader/dist/cjs.js!example.less:
    Entrypoint mini-css-extract-plugin = *
    [1] /Users/danieltschinder/Documents/Github/react-tabs/node_modules/css-loader!/Users/danieltschinder/Documents/Github/react-tabs/node_modules/less-loader/dist/cjs.js!./example.less 4.17 KiB {0} [built]
        + 1 hidden module
Child mini-css-extract-plugin ../../node_modules/css-loader/index.js!../../node_modules/less-loader/dist/cjs.js!../../node_modules/react-live/react-live.css:
    Entrypoint mini-css-extract-plugin = *
       2 modules

Webpack config: https://github.com/reactjs/react-tabs/blob/master/webpack.config.js

How to reproduce:

  1. git clone https://github.com/reactjs/react-tabs.git
  2. yarn install
  3. yarn run website:build
  4. Look at examples/dist/app-xxx.css and see that it does not contain the css from react-live/react-live.css (easily discoverable by searching for .prism-code { which should be included)
@danez
Copy link
Author

danez commented Apr 18, 2018

Even when I remove the codesplitting it still does not work.

@alexander-akait
Copy link
Member

@danez thanks for issue, in near future i will investigate

@alexanderchr
Copy link

Noticing this too. No styles from external packages appear in the bundled css.

@daru23
Copy link

daru23 commented Apr 23, 2018

I am having the same issue. If you find the solution, could you please update. Thanks a lot.

@alexander-akait
Copy link
Member

Looks like a bug, feel free to investigate and fix PR

@alexanderchr
Copy link

I've investigated it further and the issue seems to appear when using import statements to load styles from packages with scoped names. Using require or loading styles from non-scoped packages seems to work fine.

@sokra
Copy link
Member

sokra commented Apr 27, 2018

react-live has sideEffects: false in it's package.json. This flags all modules as side effect free. But this is incorrect as css files have a side effect (adding styles to the DOM). As you are not using any export of the imported css webpack flags the css file as unneeded and doesn't include it in the css bundle.

Instead the package.json should do sideEffects: [ "*.css" ].

You can also override this in your module.rules: { test: /\.css$/, include: path.resolve(__dirname, "node_modules/react-live", sideEffects: true }

@alexanderchr
Copy link

Looks like all the packages I tested with scoped names have ‘sideEffects: false’ in their package.json. The ones without scope don’t. So my conclusion above is invalid - thank you for the clarification sokra. I think this issue can be closed.

Out of curiousity: why does ‘require’ work in this case? Do using it imply that you desire side effects?

@dcporter
Copy link

@sokra Appreciated that this advice represents the status quo, but I would like to bring up (prev) that ideally the developer shouldn't have to specify this in their package.json — the import syntax that's being being used — import '/modules/my-module.js'; — is referred to by MDN as "Import a module for its side effects only", and there's no other purpose for such an import. Since the developer is already specifying that they're importing these files for their side effects, it's redundant and fragile to have to also specify them in package.json.

@jmbothe
Copy link

jmbothe commented Nov 6, 2018

@sokra you are a legend! Fixed this exact issue for me. Wish I had read your comment an hour ago :)

@devongovett
Copy link

@sokra The behavior here is quite problematic for libraries that wish to support tree shaking unused CSS. Imagine a package with multiple components. Importing one of the components should include only the CSS for that component, not the entire package.

import {Button} from 'my-great-button';

my-great-button has an index.js file with the following:

export * from './Button';
export * from './FancyButton';

Importing Button should not include the CSS for FancyButton as well, but with "sideEffects: ["*.css"] it would.

CSS should only be included if the JS that imports it is included. i.e. if FancyButton is tree-shaken, so should whatever CSS FancyButton imports. I wouldn't necessarily think of CSS itself as a side effect: the effect is really in the module that imports the CSS.

@alexander-akait
Copy link
Member

Answer above #102 (comment)

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

8 participants