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

feat: support for multiple entry files #264

Merged

Conversation

chmelevskij
Copy link
Contributor

@chmelevskij chmelevskij commented Apr 19, 2020

Continuing on the work of #230

This is a basic example for support for bundling multiple css files based on the number of entries. Rationale being that if you are using js modules and might have multiple entries you might not need to load all of the css at once. This would be handled in some user chosen way.

There is one question though, how to handle the extract: "some/custom/path" in such cases:

  1. Put files relative to the entry points. This most likely will brake some people code.
  2. In case of extract being a string fall back to single css file output.

Would need more input from the team on this.

@codecov
Copy link

codecov bot commented Apr 19, 2020

Codecov Report

Merging #264 into master will increase coverage by 0.35%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #264      +/-   ##
==========================================
+ Coverage   92.47%   92.83%   +0.35%     
==========================================
  Files          10       10              
  Lines         319      335      +16     
  Branches      112      115       +3     
==========================================
+ Hits          295      311      +16     
  Misses         23       23              
  Partials        1        1              
Impacted Files Coverage Δ
src/index.js 99.12% <100.00%> (+0.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e71f75...2ff9c36. Read the comment docs.

@chmelevskij chmelevskij force-pushed the feat/support-multiple-entry-points branch from cd989f6 to 940b216 Compare April 19, 2020 15:31
@chmelevskij chmelevskij marked this pull request as ready for review April 19, 2020 15:31
@chmelevskij chmelevskij force-pushed the feat/support-multiple-entry-points branch 2 times, most recently from 5dd0e27 to 840778c Compare April 19, 2020 16:25
@chmelevskij chmelevskij changed the title WIP: support for multiple entry files feat: support for multiple entry files Apr 19, 2020
@chmelevskij chmelevskij force-pushed the feat/support-multiple-entry-points branch 3 times, most recently from 5326e94 to 81b9f90 Compare April 19, 2020 17:12
@chmelevskij
Copy link
Contributor Author

Made so that it concatenates the css file if extract string is provided. The only caveat being that it overwrites it multiple times.

@chmelevskij chmelevskij force-pushed the feat/support-multiple-entry-points branch from 81b9f90 to 45591f2 Compare April 19, 2020 17:17
@SASUKE40 SASUKE40 self-requested a review April 20, 2020 03:17
@SASUKE40
Copy link
Collaborator

There were some warning messages after executing yarn test.

image

Could you remove the warning messages and add multiEntry option usage to README.md documentation?

@chmelevskij chmelevskij force-pushed the feat/support-multiple-entry-points branch 2 times, most recently from d060cbb to 0d88220 Compare April 22, 2020 20:40
Bundle multiple bundles for multiple input entries.
@chmelevskij chmelevskij force-pushed the feat/support-multiple-entry-points branch from 0d88220 to 2ff9c36 Compare April 23, 2020 06:45
@chmelevskij
Copy link
Contributor Author

@SASUKE40 fixed up the warnings, but will do some refactoring and will add more tests for this use case. There are quite a few different cases with this so would be good to cover them.

Also removed the multiEntry option, since it can be inferred from the input itself. Will add some info about that as well once tests are there.

@SASUKE40
Copy link
Collaborator

Looks great, thanks a lot!

src/index.js Outdated Show resolved Hide resolved
src/index.js Outdated Show resolved Hide resolved
@SASUKE40 SASUKE40 merged commit 3ea986c into egoist:master Apr 23, 2020
@egoist
Copy link
Owner

egoist commented Apr 23, 2020

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@peter-mouland
Copy link

epic - living the OSS dream 🎉

@coxmi
Copy link
Contributor

coxmi commented Apr 23, 2020

This latest commit (3ea986c) seems to break for me, and is outputting empty css files. Is this a regression? Any help would be appreciated.

rollup.config.js:

const test = {
    input : 'src/pages/test.js',
    output : {
        dir: 'dist',
        format: 'cjs',
        sourcemap: 'inline'
    },
    plugins : [
        postcss({
            inject : false,
            extract: 'test.css',
            minimize : false,
            sourceMap : true
        })
    ]
}

export default [ test ]

src/pages/test.js:

import '../styles/main.less'

src/styles/main.less:

@color:blue;

body {
	background:@color;
}

the resulting file is zero bytes, or when a sourcemap is used, it's


/*# sourceMappingURL=test.css.map */

Thanks!
Michael

@emjaksa
Copy link

emjaksa commented Apr 24, 2020

This latest commit (3ea986c) seems to break for me, and is outputting empty css files. Is this a regression? Any help would be appreciated.

rollup.config.js:

const test = {
    input : 'src/pages/test.js',
    output : {
        dir: 'dist',
        format: 'cjs',
        sourcemap: 'inline'
    },
    plugins : [
        postcss({
            inject : false,
            extract: 'test.css',
            minimize : false,
            sourceMap : true
        })
    ]
}

export default [ test ]

src/pages/test.js:

import '../styles/main.less'

src/styles/main.less:

@color:blue;

body {
	background:@color;
}

the resulting file is zero bytes, or when a sourcemap is used, it's


/*# sourceMappingURL=test.css.map */

Thanks!
Michael

I am experiencing the same issue after updating to v3.1.0

SASUKE40 added a commit that referenced this pull request Apr 24, 2020
@SASUKE40
Copy link
Collaborator

@MichaelAnd @emjaksa
Reverted, use v3.1.1.

Copy link
Collaborator

@SASUKE40 SASUKE40 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of test snapshots leads to pass test cases about extract empty files.

@chmelevskij
Copy link
Contributor Author

This latest commit (3ea986c) seems to break for me, and is outputting empty css files. Is this a regression? Any help would be appreciated.

rollup.config.js:

const test = {
    input : 'src/pages/test.js',
    output : {
        dir: 'dist',
        format: 'cjs',
        sourcemap: 'inline'
    },
    plugins : [
        postcss({
            inject : false,
            extract: 'test.css',
            minimize : false,
            sourceMap : true
        })
    ]
}

export default [ test ]

src/pages/test.js:

import '../styles/main.less'

src/styles/main.less:

@color:blue;

body {
	background:@color;
}

the resulting file is zero bytes, or when a sourcemap is used, it's


/*# sourceMappingURL=test.css.map */

Thanks!
Michael

Was this happening with only less and transpiled css?

@coxmi
Copy link
Contributor

coxmi commented Apr 24, 2020

scss files didn't work either, with a simple:

$color: red;
body {
    background: $color;
}

(I'm on node v13.7.0, if that helps)

@chmelevskij
Copy link
Contributor Author

@MichaelAnd just had a quick look at it. So the issue isn't the transpiled files. I think this happens with cases where css is not imported as named module E.G.import 'hello.css' doesn't render correctly.

Will have a look this 😉

@chmelevskij
Copy link
Contributor Author

The fix for single bundle won't be to complicated. But there are few cases which would need to cover better when there are mixed imports like:

import baz from './style.css'
import 'foo.css'

@evdama
Copy link

evdama commented Apr 25, 2020

My setup is broken again too, it worked until about a week ago, then stopped working, then worked until yesterday now, with v3.1.1 it's broken again. I was always on the latest version and didn't change anything on my codebase over the last two weeks.

I'm using rollup-plugin-postcss with Tailwind, where I have my Tailwind config in a file with some top imports where I include Tailwind utils etc.

// tailwind.css

@import "tailwindcss/base";
@import "tailwindcss/components";
@import './components';
@import "tailwindcss/utilities";
@import './edm-utilities';

... more config ...

The second step is that this file is then included in my javascript code like this inside server.js

// server.js

import './css/tailwind.css'
import * as sapper from '@sapper/server'
import express from 'express'

... more code and config ...

And then I build it with rollup and this is where it stopped working with v3.1.1 again even though it worked for the last two or so days before v3.1.1 again.

Here is my rollup conf with the extract line in line 186: extract: 'static/global.css',
https://gist.github.com/b1e2f38ddf4898a1343c140c075c12f3

With v3.1.1 it doesn't write/output the global.css file anymore meaning I don't have any CSS at all. This worked until two days ago, then I upgraded to the latest again and now it's broken (no global.css).

@SASUKE40
Copy link
Collaborator

My setup is broken again too, it worked until about a week ago, then stopped working, then worked until yesterday now, with v3.1.1 it's broken again. I was always on the latest version and didn't change anything on my codebase over the last two weeks.

I'm using rollup-plugin-postcss with Tailwind, where I have my Tailwind config in a file with some top imports where I include Tailwind utils etc.

// tailwind.css

@import "tailwindcss/base";
@import "tailwindcss/components";
@import './components';
@import "tailwindcss/utilities";
@import './edm-utilities';

... more config ...

The second step is that this file is then included in my javascript code like this inside server.js

// server.js

import './css/tailwind.css'
import * as sapper from '@sapper/server'
import express from 'express'

... more code and config ...

And then I build it with rollup and this is where it stopped working with v3.1.1 again even though it worked for the last two or so days before v3.1.1 again.

Here is my rollup conf with the extract line in line 186: extract: 'static/global.css',
https://gist.github.com/b1e2f38ddf4898a1343c140c075c12f3

With v3.1.1 it doesn't write/output the global.css file anymore meaning I don't have any CSS at all. This worked until two days ago, then I upgraded to the latest again and now it's broken (no global.css).

Could you find global.css related to your bundle root ?
https://github.com/egoist/rollup-plugin-postcss#extract-css

@evdama
Copy link

evdama commented Apr 25, 2020

Could you find global.css related to your bundle root ?
https://github.com/egoist/rollup-plugin-postcss#extract-css

My setup hasn't changed for about 8 months now, just the npm packes got upgraded on a regular. So a week ago using path my setup broke with v2.8.2, then for some reason it started working again (global.css was written), then/now with 3.1.1 it's broken again.

Summary: It worked until v2.8.2. which broke my setup: #250 (comment)

Then I switched to not using path at all as you can see from my rollup gist above, snippet here:

current rollup conf snippet...

      postcss({
        plugins: postcssPlugins(!dev),
        extract: 'static/global.css',     <--- this worked with the version after 2.8.2 and before 3.1.1
      }),

And now, with the current version v3.1.1 it's broken again i.e. no global.css gets written. I saw that 3.1.1 was a revert of a change, which seemed to be the one that my setup worked with :)

@SASUKE40
Copy link
Collaborator

Could you find global.css related to your bundle root ?
https://github.com/egoist/rollup-plugin-postcss#extract-css

My setup hasn't changed for about 8 months now, just the npm packes got upgraded on a regular. So a week ago using path my setup broke with v2.8.2, then for some reason it started working again (global.css was written), then/now with 3.1.1 it's broken again.

Summary: It worked until v2.8.2. which broke my setup: #250 (comment)

Then I switched to not using path at all as you can see from my rollup gist above, snippet here:

current rollup conf snippet...

      postcss({
        plugins: postcssPlugins(!dev),
        extract: 'static/global.css',     <--- this worked with the version after 2.8.2 and before 3.1.1
      }),

And now, with the current version v3.1.1 it's broken again i.e. no global.css gets written. I saw that 3.1.1 was a revert of a change, which seemed to be the one that my setup worked with :)

Try to downgrade the plugin, I would solve it ASAP in v3.

yarn add rollup-plugin-postcss@2 -dev

@evdama
Copy link

evdama commented Apr 25, 2020

Try to downgrade the plugin, I would solve it ASAP in v3.

yarn add rollup-plugin-postcss@2 -dev

So I did. I'm now on version v2.9.0 and it works again i.e. my global.css is written and my website has its styles again. Downgrading rollup-plugin-postcss was all I did, no other change at all...

So this works with v2.9.0 but breaks my site with v3.1.1

... other rollup code see gist above...

      postcss({
        plugins: postcssPlugins(!dev),
        extract: 'static/global.css',     <--- works with v2.9.0 but breaks my site with v3.1.1
      }),

... other rollup.config code see gist above...

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.

7 participants