Skip to content

Commit

Permalink
bug #930 Fix Encore.copyFiles() when copying images/fonts (Lyrkan)
Browse files Browse the repository at this point in the history 
This PR was merged into the main branch.

Discussion
----------

Fix Encore.copyFiles() when copying images/fonts

This PR fixes an issue that prevented `Encore.copyFiles()` from working properly when copying files that matched one of the default `asset/resource` rules.

Note that it has two drawbacks:
* it introduces what could be considered as a small breaking change for users calling `configureLoaderRule('images', ...)` or `configureLoaderRule('fonts', ...)` since it adds a new `oneOf` at the top-level of each rule
* if an user create a new rule that also matches files handled by `Encore.copyFiles(...)` they'll have to use the same `oneOf`/`resourceQuery` pattern to avoid conflicts

Closes #894

Commits
-------

f21427e Fix Encore.copyFiles() when copying images/fonts
  • Loading branch information
@weaverryan
weaverryan committed Feb 19, 2021
2 parents 17d8591 + f21427e commit baf45d3
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 17 deletions.
15 changes: 11 additions & 4 deletions lib/config-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class ConfigGenerator {

return buffer + `
const context_${index} = require.context(
'${stringEscaper(`!${copyFilesLoaderConfig}!${copyFrom}`)}',
'${stringEscaper(`!!${copyFilesLoaderConfig}!${copyFrom}?copy-files-loader`)}',
${!!entry.includeSubdirectories},
${entry.pattern}
);
Expand Down Expand Up @@ -269,9 +269,16 @@ class ConfigGenerator {
ruleCallback,
{
test: testRegex,
type: ruleOptions.type,
generator: generatorOptions,
parser: parserOptions,
oneOf: [
{
resourceQuery: /copy-files-loader/,
type: 'javascript/auto',
},{
type: ruleOptions.type,
generator: generatorOptions,
parser: parserOptions
}
]
},
);

Expand Down
24 changes: 20 additions & 4 deletions lib/plugins/manifest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const manifestKeyPrefixHelper = require('../utils/manifest-key-prefix-helper');
* @return {void}
*/
module.exports = function(plugins, webpackConfig) {
const manifestPluginOptions = {
let manifestPluginOptions = {
seed: {},
basePath: manifestKeyPrefixHelper(webpackConfig),
// always write a manifest.json file, even with webpack-dev-server
Expand All @@ -36,10 +36,26 @@ module.exports = function(plugins, webpackConfig) {
}
};

manifestPluginOptions = applyOptionsCallback(
webpackConfig.manifestPluginOptionsCallback,
manifestPluginOptions
);

const userMapOption = manifestPluginOptions.map;
manifestPluginOptions.map = (file) => {
const newFile = Object.assign({}, file, {
name: file.name.replace('?copy-files-loader', ''),
});

if (typeof userMapOption === 'function') {
return userMapOption(newFile);
}

return newFile;
};

plugins.push({
plugin: new WebpackManifestPlugin(
applyOptionsCallback(webpackConfig.manifestPluginOptionsCallback, manifestPluginOptions)
),
plugin: new WebpackManifestPlugin(manifestPluginOptions),
priority: PluginPriorities.WebpackManifestPlugin
});
};
18 changes: 9 additions & 9 deletions test/config-generator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -778,13 +778,13 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);

const imagesRule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, actualConfig.module.rules);
const imagesRule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, actualConfig.module.rules).oneOf[1];
expect(imagesRule.type).to.equal('asset/resource');
expect(imagesRule.generator).to.eql({ filename: 'images/[name].[hash:8][ext]' });
expect(imagesRule.parser).to.eql({});
expect(imagesRule).to.include.keys('test', 'type', 'generator', 'parser');
expect(imagesRule).to.include.keys('type', 'generator', 'parser');

const fontsRule = findRule(/\.(woff|woff2|ttf|eot|otf)$/, actualConfig.module.rules);
const fontsRule = findRule(/\.(woff|woff2|ttf|eot|otf)$/, actualConfig.module.rules).oneOf[1];
expect(fontsRule.type).to.equal('asset/resource');
expect(fontsRule.generator).to.eql({ filename: 'fonts/[name].[hash:8][ext]' });
});
Expand All @@ -801,7 +801,7 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);

const imagesRule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, actualConfig.module.rules);
const imagesRule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, actualConfig.module.rules).oneOf[1];
expect(imagesRule.type).to.equal('asset/resource');
expect(imagesRule.generator).to.eql({ filename: 'file.[hash][ext]' });
});
Expand All @@ -818,7 +818,7 @@ describe('The config-generator function', () => {

const actualConfig = configGenerator(config);

const imagesRule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, actualConfig.module.rules);
const imagesRule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, actualConfig.module.rules).oneOf[1];
expect(imagesRule.parser).to.eql({ dataUrlCondition: { maxSize: 3000 } });
});

Expand Down Expand Up @@ -1122,22 +1122,22 @@ describe('The config-generator function', () => {

it('configure rule for "images"', () => {
config.configureLoaderRule('images', (loaderRule) => {
loaderRule.generator.filename = 'dirname-images/[hash:42][ext]';
loaderRule.oneOf[1].generator.filename = 'dirname-images/[hash:42][ext]';
});

const webpackConfig = configGenerator(config);
const rule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, webpackConfig.module.rules);
const rule = findRule(/\.(png|jpg|jpeg|gif|ico|svg|webp)$/, webpackConfig.module.rules).oneOf[1];

expect(rule.generator.filename).to.equal('dirname-images/[hash:42][ext]');
});

it('configure rule for "fonts"', () => {
config.configureLoaderRule('fonts', (loader) => {
loader.generator.filename = 'dirname-fonts/[hash:42][ext]';
loader.oneOf[1].generator.filename = 'dirname-fonts/[hash:42][ext]';
});

const webpackConfig = configGenerator(config);
const rule = findRule(/\.(woff|woff2|ttf|eot|otf)$/, webpackConfig.module.rules);
const rule = findRule(/\.(woff|woff2|ttf|eot|otf)$/, webpackConfig.module.rules).oneOf[1];

expect(rule.generator.filename).to.equal('dirname-fonts/[hash:42][ext]');
});
Expand Down
42 changes: 42 additions & 0 deletions test/functional.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2480,6 +2480,48 @@ module.exports = {
done();
});
});

it('Does not prevent from setting the map option of the manifest plugin', (done) => {
const config = createWebpackConfig('www/build', 'production');
config.addEntry('main', './js/no_require');
config.setPublicPath('/build');
config.copyFiles({
from: './images',
pattern: /symfony_logo\.png/,
includeSubdirectories: false
});

config.configureManifestPlugin(options => {
options.map = (file) => {
return Object.assign({}, file, {
name: `${file.name}.test`,
});
};
});

testSetup.runWebpack(config, (webpackAssert) => {
expect(config.outputPath).to.be.a.directory()
.with.files([
'entrypoints.json',
'runtime.js',
'main.js',
'manifest.json',
'symfony_logo.png'
]);

webpackAssert.assertManifestPath(
'build/main.js.test',
'/build/main.js'
);

webpackAssert.assertManifestPath(
'build/symfony_logo.png.test',
'/build/symfony_logo.png'
);

done();
});
});
});

describe('entrypoints.json & splitChunks()', () => {
Expand Down

0 comments on commit baf45d3

Please sign in to comment.