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(index): add checkCssChunk option and cssLinkHref function #256

Closed
wants to merge 4 commits into from
Closed

Conversation

l5oo00
Copy link

@l5oo00 l5oo00 commented Aug 29, 2018

give a chance to modify the css href by "mainTemplate.hooks.localVars" for another css resource manager.

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

I just want to use this plugin to extract and load CSS files, but for CSS file dependency management, I need additional plugins to support.

So I need a chance to modify the css href, and do not check by cssChunks variable.

mainTemplate.hooks.localVars.tap(pluginName, (source, chunk, hash) => {
  return Template.asString([
    source,
    '(function () {',
    Template.indent([
      'var ori = cssLinkHref;',
      'cssLinkHref = function (chunkId) {',
      Template.indent([
        '',
        '// do something by another css resource manager & maybe return other href',
        '',
        'return ori.call(this, chunkId);',
      ]);
      '};',
    ]),
    '})();'
  ]);
});

Breaking Changes

no

Additional Info

no

give a chance to modify the css href by "mainTemplate.hooks.localVars"
for another css resource manager
@jsf-clabot
Copy link

jsf-clabot commented Aug 29, 2018

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Need tests

@michael-ciniawsky michael-ciniawsky changed the title refactor: give a chance to modify the css href feat(index): add checkCssChunk && forceHookRuntime options Aug 29, 2018
@l5oo00
Copy link
Author

l5oo00 commented Aug 30, 2018

This change is for the runtime chunk, not for css files.

I think it would be enough if the existing test cases passed, and I didn't see any existing tests for the runtime chunk.

So need tests?

@alexander-akait
Copy link
Member

@l5oo00 yes

@l5oo00 l5oo00 changed the title feat(index): add checkCssChunk && forceHookRuntime options feat(index): add checkCssChunk option Sep 4, 2018
@l5oo00
Copy link
Author

l5oo00 commented Sep 4, 2018

Remove forceHookRuntime option because it relies on the requireExtensions hook inside webpack, so temporarily remove it.

// the `requireExtensions` hook in MainTemplate.js
this.hooks.requireExtensions.tap("MainTemplate", (source, chunk, hash) => {
    const buf = [];
    const chunkMaps = chunk.getChunkMaps();
    // Check if there are non initial chunks which need to be imported using require-ensure
    if (Object.keys(chunkMaps.hash).length) {
        buf.push("// This file contains only the entry chunk.");
        buf.push("// The chunk loading function for additional chunks");
        buf.push(`${this.requireFn}.e = function requireEnsure(chunkId) {`);
        buf.push(Template.indent("var promises = [];"));
        buf.push(
            Template.indent(
                this.hooks.requireEnsure.call("", chunk, hash, "chunkId")
            )
        );
        buf.push(Template.indent("return Promise.all(promises);"));
        buf.push("};");
    }

    // ...
});

And, @evilebottnawi added test for checkCssChunk option.

@l5oo00 l5oo00 changed the title feat(index): add checkCssChunk option feat(index): add checkCssChunk option and cssLinkHref function Sep 10, 2018
@aleen42
Copy link

aleen42 commented Oct 19, 2018

Any progress? I have found that if we use css-split-webpack-plugin combined with mini-css-extract-plugin, requireEnsure should throw an error as the prior plugin has split them into different files like "xxx-1.css", "xxx-2.css", as "xxx.css" has not been existed already. This option is what I really need to work around this checking

@alexander-akait
Copy link
Member

This PR break work mini-css-extract-plugin in many many cases, it is not right solution and contains breaking change

@aleen42
Copy link

aleen42 commented Oct 19, 2018

So any proper way to work around such a situation, where I need the plugin not to check CSS chunks when Webpack bootstrap.

@aleen42
Copy link

aleen42 commented Oct 24, 2018

@evilebottnawi Or any plan for doing the job of css-split-webpack-plugin within mini-css-extract-plugin?

@lwr
Copy link

lwr commented Oct 29, 2018

As we has already generate html to load css statically, we don't need the bootstrap css loading injection (which is after // mini-css-extract-plugin CSS loading)

So we have workarounds like this

config = {
    // ...
    plugins: [
        // ...
        Object.assign(new MiniCssExtractPlugin({filename : config.output.filename.replace(/\.js$/, '.css')}), {
            // extracted css is preloaded, no bootstrap injection required
            // simultaneously workaround the chunk loading problem, see https://github.com/webpack-contrib/mini-css-extract-plugin/pull/256
            getCssChunkObject() { return {} },
        })
        // ...
    ]
}

plugin itself should provider this option (preloaded css statically) in it's own config


updated 2021-08-13

the hack not worked in 2.2.0 now

@lwr
Copy link

lwr commented Aug 13, 2021

the above hack is never worked now (version 2.2.0)

since getCssChunkObject is now local and can not be overridden

any other option?

now we have to patch source to remove the whole webpack/runtime/css loading for __webpack_require__.f.miniCss

lwr referenced this pull request Aug 13, 2021
@lwr
Copy link

lwr commented Aug 13, 2021

the new workaround to remove __webpack_require__.f.miniCss

config = {
    // ...
    plugins: [
        new MiniCssExtractPlugin({/* ... */}), { // remove `__webpack_require__.f.miniCss`
            apply(compiler) {
                compiler.hooks.thisCompilation.tap('remove require.f.miniCss', compilation => {
                    compilation.hooks.afterRuntimeRequirements.tap('remove require.f.miniCss', () => {
                        [...compilation.modules].find(x => x.name === 'css loading').generate = () => null;
                    });
                });
            },
        }
    ]
}

@aleen42
Copy link

aleen42 commented Sep 13, 2021

the new workaround to remove __webpack_require__.f.miniCss

config = {
    // ...
    plugins: [
        new MiniCssExtractPlugin({/* ... */}), { // remove `__webpack_require__.f.miniCss`
            apply(compiler) {
                compiler.hooks.thisCompilation.tap('remove require.f.miniCss', compilation => {
                    compilation.hooks.afterRuntimeRequirements.tap('remove require.f.miniCss', () => {
                        [...compilation.modules].find(x => x.name === 'css loading').generate = () => null;
                    });
                });
            },
        }
    ]
}

#832

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

Successfully merging this pull request may close these issues.

6 participants