laravel-mix / laravel-mix

The power of webpack, distilled for the rest of us.
MIT License
5.26k stars 806 forks source link

extractVueStyles breaks multiple css files processing #425

Closed terion-name closed 7 years ago

terion-name commented 7 years ago

Mix:

const mix = require('laravel-mix');

mix.options({
  extractVueStyles: true
});

const stylusOptons = {
  paths: ['node_modules', 'resources/assets/stylus'],
  'include css': true,
  'resolve url': true
};

mix.setPublicPath('./public/assets');

mix.stylus('resources/assets/stylus/home.styl', 'css', stylusOptons);

mix
    .stylus('resources/assets/stylus/vote/main.styl', 'css/vote', stylusOptons)
    .stylus('resources/assets/stylus/vote/alter.styl', 'css/vote', stylusOptons)
    .stylus('resources/assets/stylus/vote/order.styl', 'css/vote', stylusOptons)
    .stylus('resources/assets/stylus/vote/open.styl', 'css/vote', stylusOptons);

mix
    .sourceMaps()
    .js('resources/assets/js/app.js', 'js')
    .js('resources/assets/js/ui.js', 'js')
    .extract(['vue', 'vuex', 'vue-resource'])
    .stylus('resources/assets/stylus/app.styl', 'css', stylusOptons)
    .copy('resources/assets/images', 'public/assets/images', false)
    .copy('resources/assets/fonts', 'public/assets/fonts', false);

Enabling extractVueStyles is leading to situation, when extracted styles are placed in each and every outputted css.

Probably extractVueStyles should accept not a bool, but a path where to extract styles, or extracting should place styles near js file, from what they are extracted (e.g. app.js.css). At current point this simply doesn't work as expected and breaks everything

JeffreyWay commented 7 years ago

This is fixed on master now. You can either stick with:

mix.options({
    extractVueStyles: true
});

Or, you can provide an explicit path:

mix.options({
    extractVueStyles: 'public/css/vue-css.css'
});
terion-name commented 7 years ago

@JeffreyWay seems to work, but only if one app is built.

e.g. in this case:

mix
    .sourceMaps()
    .stylus('resources/assets/stylus/app.styl', 'css', stylusOptons)
    .js('resources/assets/js/app.js', 'js')
    .js('resources/assets/js/ui.js', 'js')
    .extract(['vue', 'vuex', 'vue-resource', 'vue-router', 'laravel-echo', 'socket.io-client', 'vue-multiselect', 'isotope', 'vueisotope', 'isotope-packery', 'vue-chartjs', 'vue-color', /*'vue-awesome',*/ 'vue-paginate'])

Styles will be extracted from ui.js and placesd to app.css (which, btw, is overwrited, so contents of app.styl is ignored — not a big deal, but if it is possible to append extracted output — it would be great).

Is it possible to make it make separate files for each js? In this case: app.js => app.css and ui.js => ui.css?

terion-name commented 7 years ago

btw: https://github.com/JeffreyWay/laravel-mix/pull/431

terion-name commented 7 years ago

@JeffreyWay also two moments:

  1. mix
    .sourceMaps()
    .js('resources/assets/js/app.js', 'js')
    .stylus('resources/assets/stylus/app.styl', 'css', stylusOptons)

    If I remove stylus call (because app.styl contents is overwritten and it makes no sense) extracted styles are not written anywhere, app.css is not created.

  2. Styles from library imports are not extracted. E.g. I have a file icons.js that contains import Icon from 'vue-awesome/components/Icon.vue'. While the component is imported properly, if I enable style extracting — styles that vue-awesome/components/Icon.vue contains are not added to output stylesheet

JeffreyWay commented 7 years ago

@terion-name

If I remove stylus call (because app.styl contents is overwritten and it makes no sense) extracted styles are not written anywhere, app.css is not created.

I'm not sure what you mean. If you remove the mix.stylus() call, then it shouldn't create app.css. Why would it create app.css if you remove that?

If you just do:

mix.sourceMaps()
    .js('resources/assets/js/app.js', 'js')
    // .stylus('resources/assets/stylus/app.styl', 'css')
    .options({
        extractVueStyles: true
    });

It'll throw the Vue-specific extracted styles in public/js/vue-style.css by default. Set extractVueStyles: 'public/somewhere/else.css' to change the default.

terion-name commented 7 years ago

@JeffreyWay hmmm... I've pulled fresh from master, updated webpack config, run this again but no, look at the output in console:

2017-02-20 23 38 47

It does not create js/vue-style.css, it places extracted js contents to all css files generated from stylus

JeffreyWay commented 7 years ago

It's not creating js/vue-style.css because you have those mix.stylus() calls.

js/vue-style.css will only be created if the following two things are true:

  1. You set extractVueStyles to a boolean, instead of a file path.
  2. There are no mix.sass/less/stylus() calls.
JeffreyWay commented 7 years ago

which, btw, is overwrited, so contents of app.styl is ignored — not a big deal, but if it is possible to append extracted output — it would be great).

I'm trying to reproduce this, but can't. Vue styles are prepended to the top of the file. For example:

mix.sourceMaps()
    .js('resources/assets/js/app.js', 'js')
    .stylus('resources/assets/stylus/app.styl', 'css')
    .options({
        extractVueStyles: true
    });

With this configuration, public/css/app.css will contain all of the Stylus CSS from resources/assets/stylus/app.styl, and then any Vue extractions at the top.

terion-name commented 7 years ago

@JeffreyWay

It's not creating js/vue-style.css because you have those mix.stylus() calls.

but this was exact the problem described in the issue =) If you try to build separate styles and extract styles from vue — this is not working.

At current state explicity providing extractVueStyles: 'css/vue.css' leads to even more strange behaviour: vue.css is created. it is correct and has all the imports. But all other css-files, that should contain results of separate stylus outputs — are overwritten:

app_css_-_app_-____freelance_freelance__1991_vdmk_app__-_phpstorm_2017_1_eap

While extractVueStyles: true can be explained that it appends vue styles to other (but this is still a problem when you have multiple stylesheets and no, it overwrites, I've tested a lot, dunno why), but in any case if extractVueStyles: 'css/vue.css' is provided — then 100% vue styles should not be appended to any other styles

terion-name commented 7 years ago

Vue styles are prepended to the top of the file

btw this should be opposite I think. If you have an app.styl — it will containt some global styles, resets, layouting, etc. And js contains components that are layered on top by priority hierarchy. So following the logic, extracted styles should be appended, not prepended — to have more weight

JeffreyWay commented 7 years ago

but this was exact the problem described in the issue =) If you try to build separate styles and extract styles from vue — this is not working.

The main issue was that extracted Vue styles were being appended to all mix.stylus() output paths. That part was fixed. There is an issue with multiple mix.js() calls, so stick with one for now, if you want to use this feature. Most apps shouldn't need multiple mix.js() calls.

if extractVueStyles: 'css/vue.css' is provided — then 100% vue styles should not be appended to any other styles

It's not for me, so if you can give me basic reproducible steps in a fresh Laravel app, that would be perfect. Because right now, if I have:

mix.sourceMaps()
    .js('resources/assets/js/app.js', 'js')
    .stylus('resources/assets/stylus/app.styl', 'css')
    .options({
        extractVueStyles: 'css/vue.css'
    })

...then, public/css/vue.css will contain all of the extracted Vue styling, and public/css/app.css will only contain the relevant CSS from resources/assets/stylus/app.styl

terion-name commented 7 years ago

very strange things are going on. give me several minutes, I'll make a demo app

terion-name commented 7 years ago

@JeffreyWay while experimenting with both new and old app, I've figured out the following:

  1. vue styles are really prepended, not overwritten (but I am still certain, that thay should be appended, not prepended)
  2. if multiple styles are called and extractVueStyles is set to true - vue styles will be added to the first style called (I think this should be mentioned in docs to save time of other people who may be confused with this)

And in the end I've figured out that... I need more to sleep. So many hours of tests and bothering you just to see this in webpack config:

sass: vueExtractTextPlugin.extract({
            use: 'css-loader!sass-loader?indentedSyntax',
            fallback: 'vue-style-loader'
          }),
stylus: plugins.ExtractTextPlugin.extract({
            use: 'css-loader!stylus-loader?paths[]=node_modules&paths[]=resources/assets/stylus&include css&resolve url',
            fallback: 'vue-style-loader'
          }),

just incorrect (old) plugin for stylus. this was leading to styles polution. what a shame.

In the end we have that everything works, except styles cannot be properly extracted from several vue apps. But I don't know is there a proper solution to support several apps in this case...

And, again, it would be probably append, not prepend extracted styles when extractVueStyles is set to true.

Thank you, Jeff, and sorry for wasting your time :(

JeffreyWay commented 7 years ago

Okay, glad you got it all sorted out. I'm going to make a note about the prepend vs append, and of course the multiple mix.js() calls issue, which I'm not entirely sure how to handle just yet.

terion-name commented 7 years ago

@JeffreyWay about multiple mix.js() calls — maybe it will be easier to make possible to use several mix-files?

e.g. npm run dev --mix=mymix

In most cases developer will have no need to build several scripts or style sets. And the way mix is built — it is aiming the most common case in most easy way. And the problem described can appear in cases of some specific complex apps. I don't see an easy solution to this problem instead of two ways:

  1. multiple mix files (webpack.mix.js, app.mix.js, admin.mix.js) — this seems to be the easiest and clear, but will force to call several build commands and will pollute root directory
  2. Introduce some isolated scopes in mix, that will all run from white paper, e.g.
    mix.scope((mix) => {
    mix.js('app.js').stylus('app.stylus');
    });
    mix.scope((mix) => {
    mix.js('admin.js').stylus('admin.stylus');
    });
    /// etc

    This will keep root dir clean, you will still use one command to build, but seems to be harder to implement and also can lead to mix-file overbloat (but not a big deal, I think, can be handled with imports)

iflamed commented 7 years ago

@terion-name @JeffreyWay I have the same problem with multiple mix.js file. I also have two app, one is spa for admin, and another app is for users with bootstrap, not a spa. Maybe mix.scope() is the best way to solve this problem.

Now when I build with laravel-mix, both app have to include the vendor.js file. However, the bootstrap app for users isn't using the vue stack. So if there is a way to split this two app?

The code of my webpack.mix.js

const { mix } = require('laravel-mix');

/*
 |--------------------------------------------------------------------------
 | Mix Asset Management
 |--------------------------------------------------------------------------
 |
 | Mix provides a clean, fluent API for defining some Webpack build steps
 | for your Laravel application. By default, we are compiling the Sass
 | file for the application as well as bundling up all the JS files.
 |
 */

mix.options({
        extractVueStyles: 'css/vue.css'
    })
    .js('resources/assets/js/app.js', 'public/js') // for admin, use the vue stack
    .js('resources/assets/js/main.js', 'public/js') // for public users, dosn't using the vue stack
    .sass('resources/assets/sass/frontend.scss', 'public/css');

if (mix.config.inProduction) {
    mix.extract([
            'vue', 'quill', 'lodash', 'axios',
            'vue-router', 'element-ui', 'vuex'
        ])
        .version();
}
feryardiant commented 5 years ago

Experiencing the same situation when I have two separated apps say, main and admin. Both are using Vue stack and I need to extract the css from two JS entries.

mix.js('resources/js/admin.js', 'public/js')
  .js('resources/js/main.js', 'public/js')

Since the extractVueStyles is expecting a file name for the extraced css and it uses extract-text-webpack-plugin, so I tried to pass a pattern instead of exact filename like this

mix.options({
    extractVueStyles: 'css/[name].css'
})

and I got this

            Asset      Size  Chunks             Chunk Names
css//js/admin.css  53.2 KiB       2  [emitted]  /js/admin
 css//js/main.css  52.3 KiB       3  [emitted]  /js/main
     /js/admin.js  1.65 KiB       2  [emitted]  /js/admin
      /js/main.js  1.61 KiB       3  [emitted]  /js/main
  /js/manifest.js  1.42 KiB       0  [emitted]  /js/manifest
    /js/vendor.js   179 KiB       1  [emitted]  /js/vendor

I wonder if it possible to pass a function or object to extractVueStyles so we could do something like this

mix.options({
  extractVueStyles: {
    filename (getPath) {
      return getPath('[name].css').replace('js/', 'css/')
    }
  }
})

// OR

mix.options({
  extractVueStyles (getPath) {
    return getPath('[name].css').replace('js/', 'css/')
  }
})

Toughts?

Erutan409 commented 4 years ago

@feryardiant I'll be extrapolating this into a plugin at some point, but this is what I came up with:

// configure vue style extraction
tap(null, () => {

    mix.options({
        extractVueStyles: {
            filename(getPath) {
                return getPath('css/vue[name].css').replace(/^(.*)vue\/js\/(.*)\.css$/i, '$1$2-vue.css');
            }
        }
    });

    // override method to allow for filename method
    Mix.listen('configReady', () => {

        const vue = Mix.components.get('js').vue;

        Helper.proxyMethodAfter(vue, 'extractFileName', filename => {

            const config = Config.extractVueStyles;

            if (typeof config !== 'object' || typeof config.filename !== 'function') {
                return filename;
            }

            return config;

        }, true);

    });

});

Some functionality to help me "intercept" the method:

const Assert = require('assert');

/**
 * Various helpers
 */
class Helpers {

    /**
     * Proxy a class method after it executes.
     *
     * @param {Object} obj
     * @param {String} method
     * @param {Function} callable
     * @param {Boolean} pass_callback
     */
    static proxyMethodAfter(obj, method, callable, pass_callback = false) {

        Assert(typeof obj === 'object', 'Expecting valid object.');
        Assert(typeof method === 'string', 'Expecting valid method string.');
        Assert(typeof callable === 'function', 'Expecting valid callback.');
        Assert(typeof obj[method] === 'function', `No valid method: ${method}`);

        const orig = obj[method];

        obj[method] = (...args) => {

            let result = orig.apply(obj, args);
            let called_result = callable(result, args, obj);

            return Boolean(pass_callback) ? called_result : result;

        };

    }

}

Thanks for pointing out how to get this broken up into more manageable pieces.

andremartinsc commented 4 years ago

Experiencing the same situation when I have two separated apps say, main and admin. Both are using Vue stack and I need to extract the css from two JS entries.

mix.js('resources/js/admin.js', 'public/js')
  .js('resources/js/main.js', 'public/js')

Since the extractVueStyles is expecting a file name for the extraced css and it uses extract-text-webpack-plugin, so I tried to pass a pattern instead of exact filename like this

mix.options({
    extractVueStyles: 'css/[name].css'
})

and I got this

            Asset      Size  Chunks             Chunk Names
css//js/admin.css  53.2 KiB       2  [emitted]  /js/admin
 css//js/main.css  52.3 KiB       3  [emitted]  /js/main
     /js/admin.js  1.65 KiB       2  [emitted]  /js/admin
      /js/main.js  1.61 KiB       3  [emitted]  /js/main
  /js/manifest.js  1.42 KiB       0  [emitted]  /js/manifest
    /js/vendor.js   179 KiB       1  [emitted]  /js/vendor

I wonder if it possible to pass a function or object to extractVueStyles so we could do something like this

mix.options({
  extractVueStyles: {
    filename (getPath) {
      return getPath('[name].css').replace('js/', 'css/')
    }
  }
})

// OR

mix.options({
  extractVueStyles (getPath) {
    return getPath('[name].css').replace('js/', 'css/')
  }
})

Toughts?

Did you find a solution for this?

Arturokin12 commented 4 years ago

@JeffreyWay about multiple mix.js() calls — maybe it will be easier to make possible to use several mix-files?

e.g. npm run dev --mix=mymix

In most cases developer will have no need to build several scripts or style sets. And the way mix is built — it is aiming the most common case in most easy way. And the problem described can appear in cases of some specific complex apps. I don't see an easy solution to this problem instead of two ways:

  1. multiple mix files (webpack.mix.js, app.mix.js, admin.mix.js) — this seems to be the easiest and clear, but will force to call several build commands and will pollute root directory
  2. Introduce some isolated scopes in mix, that will all run from white paper, e.g.
mix.scope((mix) => {
   mix.js('app.js').stylus('app.stylus');
});
mix.scope((mix) => {
   mix.js('admin.js').stylus('admin.stylus');
});
/// etc

This will keep root dir clean, you will still use one command to build, but seems to be harder to implement and also can lead to mix-file overbloat (but not a big deal, I think, can be handled with imports)

¿how do i use .scope() function? i got this error:

TypeError: mix.scope is not a function

Please help