Skip to content

New option: EmitFile, similar to file-loader to allow for the plugin ***not*** to emit the assets #713

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

Closed
ferdinando-ferreira opened this issue Mar 6, 2021 · 13 comments

Comments

@ferdinando-ferreira
Copy link

  • Operating System: Any
  • Node Version: Any
  • NPM Version: Any
  • webpack Version: Any
  • mini-css-extract-plugin Version: Any

Feature Proposal

Addition of the option emitFile, similar to the one existing in other plugins like file-loader (see: https://webpack.js.org/loaders/file-loader/#emitfile

Feature Use Case

Much like in the file-loader use case, it allows for the resource to be extracted from the javascript (reducing its size) without actually creating the resource on disk, useful to when a SSR version of the javascript file is being generated

@ferdinando-ferreira
Copy link
Author

Would also satisfy #74, an older version of this same request that was abandoned back

@alexander-akait
Copy link
Member

You can just ignore CSS files using alias.resolve for SSR build

@ferdinando-ferreira
Copy link
Author

You can just ignore CSS files using alias.resolve for SSR build

I would need to study that alternative to ensure it is accurate but, for the sake of continuity I'll just assume it is correct. With that said I don't think it solves the use case being put forward here.

The idea with the proposed functionality (in a similar way with the way it worked for the file-loader equivalent option) is:

  1. To ensure that there is exactly zero difference (with regards to the extraction of the styles) between the output javascript files for the regular version and the SSR version
  2. While at the same time ensuring the only necessary configuration on webpack.config (also with regards to the extraction of the styles) happens to be
		plugins: [
			new MiniCssExtractPlugin({
				filename: '[name].css',
				emitFile: isSsr ? false : true
			})
		],

If necessary for the PR to be considered I can construct the use case where alias.resolve would be insufficient but emitFile: false would solve the problem, and for that all I need is assurance there is no other objection to adding the option.

Should I proceed pleading the case or there is a bigger issue here preventing the new option from being considered?

@alexander-akait
Copy link
Member

If necessary for the PR to be considered I can construct the use case where alias.resolve would be insufficient but emitFile: false would solve the problem, and for that all I need is assurance there is no other objection to adding the option.

Please provide case

@ferdinando-ferreira
Copy link
Author

Please provide case

An usecase was prepared at ferdinando-ferreira/mini-css-extract-plugin

Instructions to test

git clone https://github.com/ferdinando-ferreira/mini-css-extract-plugin.git mini-css-extract-plugin-emitfile
cd mini-css-extract-plugin-emitfile
npm install
cd usecase
npm install
npm run build

The example chosen was what is essentially the "Hello World" of vue when used with Single File Components. That project uses and endorses this plugin as the means to extract css to their own files and, consequently, provides with a very good usecase for the proposed option.

The source code being packed is not really important:

usecase/src/app.js

import Vue from 'vue'
import App from './App.vue'
new Vue({
  el: '#app',
  render: h => h(App)
})

usecase/src/App.vue

<template>
  <div>
    <h1>Hello World!</h1>
  </div>
</template>
<style>
  h1 {
    background: green;
  }
</style>

It has a peculiarity tho: App.vue can contain css, be it by having it inline in the <style> tag, by importing css (or scss if configured) files from other modules or by referring to other .vue single file components that have styles of their own.

The mini-css-extract-plugin is used here as a "catch-all", extracting from the javascript all of the generated css created by the interactions of these Single File Components and creating their own files.

Enters SSR: from the exact application two packed files are generated: one for use in the browser (let's call it index.js) and one for use in the server (index-ssr.js). This is made possible by compiling the exact same application with two webpack configurations, one for the client and one for the server.

usecase/config/webpack.config.js

'use strict'
module.exports = [
  require('./webpack.config.client.js'),
  require('./webpack.config.server.js')
]

The difference between the client and the server configuration are highlighted below, assuming the emitFile option in the plugin exists.

usecase/config/webpack.config.client.js

const { VueLoaderPlugin } = require('vue-loader')
const MiniCssExtractPlugin = require('../../dist/cjs.js');
module.exports = {
  mode: 'development',
  target: 'web', // <----------- target
  entry: {
    index: './src/app.js', // <----------- entry
  },
  module: {
    rules: [
      {
        test: /\.vue$/,
        use: 'vue-loader'
      },
      {
        test: /\.css$/,
        use: [
          { loader: "vue-style-loader" },
          { loader: MiniCssExtractPlugin.loader, options: { esModule: false } },
          { loader: "css-loader" }
        ]
      }
    ]
  },
  plugins: [
    new VueLoaderPlugin(),
    new MiniCssExtractPlugin({
      filename: '[name].css',
      emitFile: true // <----------- emitFile
    })
  ]
}

usecase/config/webpack.config.server.js

const { VueLoaderPlugin } = require('vue-loader')
const MiniCssExtractPlugin = require('../../dist/cjs.js');
module.exports = {
  mode: 'development',
  target: 'node', // <----------- target
  entry: {
    'index-ssr': './src/app.js',  // <----------- entry
  },
  module: {
    rules: [
      {
        test: /\.vue$/,
        use: 'vue-loader'
      },
      {
        test: /\.css$/,
        use: [
          { loader: "vue-style-loader" },
          { loader: MiniCssExtractPlugin.loader, options: { esModule: false } },
          { loader: "css-loader" }
        ]
      }
    ]
  },
  plugins: [
    new VueLoaderPlugin(),
    new MiniCssExtractPlugin({
      filename: '[name].css',
      emitFile: false // <----------- emitFile
    })
  ]
}

@ferdinando-ferreira
Copy link
Author

ferdinando-ferreira commented Mar 9, 2021

(continuing)

In the SSR case, despite being important to extract out the styles from the code (to reduce the size of the resulting file) it is also important to not generate the extracted file, as it has no use server side.

Likely because of not having experience with alias.resolve but I cannot really see how it is possible to achieve the same goal by passing that parameter in the webpack configuration file, considering the styles being included are not known at configuration time and their origin can be of any file or module linked by App.vue.

Can you show a way to achieve the same goal with alias.resolve? Otherwise I respectfully maintain the position that this option would be valuable for a real world case as demonstrated above.

@alexander-akait
Copy link
Member

You already do not extract styles for SSR:

'use strict'
module.exports = [
  // require('./webpack.config.client.js'),
  require('./webpack.config.server.js')
]

Output:

- dist
  - index-ssr.js

For client you need extract CSS because without it styles will not work

@ferdinando-ferreira
Copy link
Author

ferdinando-ferreira commented Mar 10, 2021

You already do not extract styles for SSR:

Thanks for acknowledging the proposed PR would solve the issue, because the reason it "already do not extract styles for SSR" is because the use case already uses the plugin with the emitFile option merged in, which is the whole point of preparing this use case, as you can see at

usecase/config/webpack.config.server.js#L27-L30

new MiniCssExtractPlugin({
  filename: '[name].css',
  emitFile: false // <------------- *** This is only possible due to the proposed PR ***
})

To clarify the point I prepared a second use case, this time using the plugin as it exists right now.

Instructions to test:

git clone https://github.com/ferdinando-ferreira/mini-css-extract-plugin.git mini-css-extract-plugin-emitfile
cd mini-css-extract-plugin-emitfile
npm install
cd usecase2
npm install
npm run build

You will notice that, unlike in the example using the modified plugin with the emitFile option merged in, it generates

- dist
  - index.js
  - index.css
  - index-ssr.js
  - index-ssr.css

Which is the exact issue motivating the PR and the reason I request this issue to be reopened and the PR reconsidered.

Here is the summary:

  • Without merging the PR index-ssr.css is emitted
  • With merging the PR index-ssr.css is not emitted, which is the desired outcome
  • There is no better way to prevent it from being emitted

You mentioned using alias.resolve as an alternative. Can you show a way to achieve the same goal with that option? Otherwise I respectfully maintain the position that this option would be valuable for a real world case as demonstrated above.

P.S. pinging @alexander-akait because I don't really know closed issues still send notifications

@ferdinando-ferreira
Copy link
Author

@alexander-akait: I'll retract the PR for now and submit a new issue and a new PR better explained and passing all tests

@alexander-akait
Copy link
Member

@ferdinando-ferreira just don't use this plugin for SSR, i.e.:

{
  test: /\.css$/,
  use: [
    { loader: "vue-style-loader" },
    { loader: "css-loader" }
  ]
}

@ferdinando-ferreira
Copy link
Author

just don't use this plugin for SSR

@alexander-akait: that wouldn't work because

  1. It changes the manifest, which is needed equal to the non SSR version.
  2. It generates the file on disk, which is what is trying to be avoided

@alexander-akait
Copy link
Member

It changes the manifest, which is needed equal to the non SSR version.

In this case you need extract, otherwise manifest still be different

It generates the file on disk, which is what is trying to be avoided

No, double check

@ferdinando-ferreira
Copy link
Author

No, double check

@alexander-akait: you are correct, it does not generate a separate index.css on disk because the css remains on index.js, unextracted.

See #722, resubmitted the PR, now passing the commit lint, please continue discussion there.

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

No branches or pull requests

2 participants