fix(loader): pass emitFile to the child compilation (loaderContext.emitFile)

[piecyk]

This PR contains a:

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

Motivation / Use-Case

Looks like mini-css-extract-plugin when collecting sources it's replacing module dependencies
with new CssDependency as assets are loaded before. IMHO we can collect and add it back to updated module.

Use-case for this is when using hard-source-plugin and reading from cache can't find asset
mzgoddard/hard-source-webpack-plugin#367

Breaking Changes

No breaking changes, all current test's are passing

Additional Info

[jsf-clabot]

All committers have signed the CLA.

[ooflorent]

Please use Object.assign(assets, module.buildInfo.assets) to reduce the GC stress.

[piecyk]

👍

[ooflorent]

|| {} is enough since you add assets just right after this statement

[piecyk]

Yeah 👍

[alexander-akait]

@piecyk Also please accept CLA, thanks!

[piecyk]

Thanks @ooflorent for review 👏squash and merge!

[piecyk]

Strange licence/cla Pending — Contributor License Agreement is not signed yet.
opening the link You have signed the CLA for webpack-contrib/mini-css-extract-plugin wat ?

[sokra]

Please add tests

[sokra]

Instead of assigning to module.assets use the emitFile loader api

[sokra]

It's also possible to hook into normalModuleLoader to change the emitFile method in the child compilation and pass through the call directly.

[michael-ciniawsky]

Blunt, but whtat's the current status of this PR ? :)

[alexander-akait]

Need tests

[piecyk]

Sorry for delay... thanks @sokra for tips, will try to finish it over the weekend or close it!

[surfaceowl]

@piecyk -- any update or timeline on this PR? Also there are some merge conflicts.

[alexander-akait]

@piecyk can you rebase PR?

[renchap]

It looks like this is creating a regression when using a manifest plugin (https://github.com/webdeveric/webpack-assets-manifest in my case).
Images, fonts and other assets referenced from CSS files are no longer present in manifest.json after upgrading from 0.4.2 to 0.4.3.
Is this intended and should there be a change in webpack-assets-manifest?

[Baune8D]

It looks like this is creating a regression when using a manifest plugin (https://github.com/webdeveric/webpack-assets-manifest in my case).
Images, fonts and other assets referenced from CSS files are no longer present in manifest.json after upgrading from 0.4.2 to 0.4.3.
Is this intended and should there be a change in webpack-assets-manifest?

Also seeing this change.

Some assets are completely missing from the manifest, others are still in there but key values have changed.

Example manifest entry:

"images/create/wallet.png": "images/create/wallet.png"

to

"images/create/Create.scss": "images/create/wallet.png"

[alexander-akait]

@Baune8D very strange, looks something wrong in plugin, we use standard webpack api and just allow loaders to emit

[renchap]

I created webdeveric/webpack-assets-manifest#39 about this, but I am really unsure this is a manifest plugin bug. This change must have done something that changes what is outputed, and it is not documented.

[piecyk]

Yeah, it looks like something wrong in webpack-assets-manifest Basic is trying to read the module.userRequest to get the original file name before hash ( or at least that how i understand it )

https://github.com/webdeveric/webpack-assets-manifest/blob/master/src/WebpackAssetsManifest.js#L548

[webdeveric]

I'm looking at this now to see if its something I can update on my end.

I am reading module.userRequest to get the original name and it was working fine in mini-css-extract-plugin 0.4.2 and below. Other loaders, like file-loader, currently work as expected.

When using 0.4.3, module.userRequest is set to the css file and not the asset (bg image, font, etc.).

[piecyk]

Maybe getting emitted files bit differently will solve the issue webdeveric/webpack-assets-manifest#39 (comment)

[chrisdarroch]

This change appears to have broken some assumptions in our webpack plugin, too. Our issue and exploration of the problem is written up here: https://bitbucket.org/atlassianlabs/atlassian-webresource-webpack-plugin/issues/52

After this change, a Module representing the CSS file will have a module.buildInfo.assets map, where the keys are the new filenames of the CSS's external file dependencies (images etc) and the values their Source.

Module {
  resource: 'style.css',
  buildInfo: {
    assets: {
      '12345.png': RawSource{...},
      '67890.svg': RawSource{...}
    }
  }
},
Module {
  resource: 'one.png',
  buildInfo: {}
},
Module {
  resource: 'two.svg',
  buildInfo: {}
}

Prior to this change, the asset metadata was stored directly on the Module for the external file dependency.

Module {
  resource: 'style.css',
  buildInfo: {}
},
Module {
  resource: 'one.png',
  buildInfo: {
    assets: {
      '12345.png': RawSource{...}
    }
  }
},
Module {
  resource: 'two.svg',
  buildInfo: {
    assets: {
      '67890.svg': RawSource{...}
    }
  }
}

The reason for this behaviour change is precisely because the emitFile method is now shared. The emitFile method is responsible for populating the buildInfo.assets map.

As a consequence, it's not easy to build a map of original filenames -> compiled filenames by iterating through all Modules involved in the compilation, since there is no obvious 1:1 mapping relationship between them.

If anybody has alternative suggestions for building the map of original filenames -> compiled filenames, I'm keen to hear them.

[chrisdarroch]

After reading through @renchap's @piecyk's and @webdeveric's exploration and fix in the webpack-assets-manifest plugin, I did something similar, with the same effect.

Instead of iterating through all Modules in the compilation, I instead hooked in to the normalModuleLoader and substituted the emitFile method with the following:

const assetNames = new Map();

compiler.hooks.compilation.tap('maybe a hack', compilation => {
    compilation.hooks.normalModuleLoader.tap('maybe a hack', (loaderContext, module) => {
        const { emitFile } = loaderContext;
        loaderContext.emitFile = (name, content, sourceMap) => {
            const originalName = module.userRequest;
            assetNames.set(originalName, name);

            return emitFile.call(module, name, content, sourceMap);
        };
    });
});

compiler.hooks.emit.tap('after emit', () => {
    console.log(assetNames); // logs originalFilepath -> compiledFilename
});

This does what I need it to, but as noted, it may be a hack. If there are better thoughts on how to do this better, let me know :)