Skip to content

Commit

Permalink
fix: extra and invalid paths in Webpack v5 (#249)
Browse files Browse the repository at this point in the history
* Fixing extra and invalid paths in Webpack 5

* trying the npm fix

* allowing for any hash

* removing all the extra logic
  • Loading branch information
weaverryan authored Apr 1, 2021
1 parent 4d3e935 commit ef1f875
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 47 deletions.
68 changes: 45 additions & 23 deletions lib/helpers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const { dirname, join, basename } = require('path');

const generateManifest = (compilation, files, { generate, seed = {} }) => {
let result;
if (generate) {
Expand Down Expand Up @@ -25,7 +27,13 @@ const getFileType = (fileName, { transformExtensions }) => {
};

const reduceAssets = (files, asset, moduleAssets) => {
const name = moduleAssets[asset.name] ? moduleAssets[asset.name] : asset.info.sourceFilename;
let name;
if (moduleAssets[asset.name]) {
name = moduleAssets[asset.name];
} else if (asset.info.sourceFilename) {
name = join(dirname(asset.name), basename(asset.info.sourceFilename));
}

if (name) {
return files.concat({
path: asset.name,
Expand All @@ -52,29 +60,43 @@ const reduceAssets = (files, asset, moduleAssets) => {
});
};

const reduceChunk = (files, chunk, options) =>
Array.of(...Array.from(chunk.files), ...Array.from(chunk.auxiliaryFiles || [])).reduce(
(prev, path) => {
let name = chunk.name ? chunk.name : null;
// chunk name, or for nameless chunks, just map the files directly.
name = name
? options.useEntryKeys && !path.endsWith('.map')
? name
: `${name}.${getFileType(path, options)}`
: path;
const reduceChunk = (files, chunk, options, auxiliaryFiles) => {
// auxiliary files contain things like images, fonts AND, most
// importantly, other files like .map sourcemap files
// we modify the auxiliaryFiles so that we can add any of these
// to the manifest that was not added by another method
// (sourcemaps files are not added via any other method)
Array.from(chunk.auxiliaryFiles || []).forEach((auxiliaryFile) => {
auxiliaryFiles[auxiliaryFile] = {
path: auxiliaryFile,
name: basename(auxiliaryFile),
isInitial: false,
isChunk: false,
isAsset: true,
isModuleAsset: true
};
});

return Array.from(chunk.files).reduce((prev, path) => {
let name = chunk.name ? chunk.name : null;
// chunk name, or for nameless chunks, just map the files directly.
name = name
? options.useEntryKeys && !path.endsWith('.map')
? name
: `${name}.${getFileType(path, options)}`
: path;

return prev.concat({
path,
chunk,
name,
isInitial: chunk.isOnlyInitial(),
isChunk: true,
isAsset: false,
isModuleAsset: false
});
},
files
);
return prev.concat({
path,
chunk,
name,
isInitial: chunk.isOnlyInitial(),
isChunk: true,
isAsset: false,
isModuleAsset: false
});
}, files);
};

const standardizeFilePaths = (file) => {
const result = Object.assign({}, file);
Expand Down
14 changes: 13 additions & 1 deletion lib/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,9 @@ const emitHook = function emit(

emitCountMap.set(manifestFileName, emitCount);

const auxiliaryFiles = {};
let files = Array.from(compilation.chunks).reduce(
(prev, chunk) => reduceChunk(prev, chunk, options),
(prev, chunk) => reduceChunk(prev, chunk, options, auxiliaryFiles),
[]
);

Expand All @@ -66,6 +67,17 @@ const emitHook = function emit(
typeof emitCountMap.get(join(compiler.options.output.path, name)) === 'undefined'
);

// auxiliary files are "extra" files that are probably already included
// in other ways. Loop over files and remove any from auxiliaryFiles
files.forEach((file) => {
delete auxiliaryFiles[file.path];
});
// if there are any auxiliaryFiles left, add them to the files
// this handles, specifically, sourcemaps
Object.keys(auxiliaryFiles).forEach((auxiliaryFile) => {
files = files.concat(auxiliaryFiles[auxiliaryFile]);
});

files = files.map((file) => {
const changes = {
// Append optional basepath onto all references. This allows output path to be reflected in the manifest.
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
"lint:js": "eslint --fix --cache lib test",
"lint:json": "prettier --write codecov.yml .circleci/config.yml .eslintrc",
"lint:package": "prettier --write package.json --plugin=prettier-plugin-package",
"posttest": "npm install webpack@^4.44.2",
"posttest": "node set-webpack-version.js \"^4.44.2\" && npm install",
"security": "npm audit --audit-level=moderate",
"test": "npm run test:v4",
"test": "npm run test:v4 && npm run test:v5",
"test:v4": "ava",
"test:v5": "npm install webpack@^5.0.0 --no-save && ava"
"test:v5": "node set-webpack-version.js \"^5\" && npm install && ava"
},
"files": [
"lib",
Expand Down
25 changes: 25 additions & 0 deletions set-webpack-version.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* This file sets Webpack version - both devDependencies and peerDependencies.
*
* This is, for some reason, needed with Windows and maybe npm 6. Running
* "npm install webpack -D" AND manually updating the peerDependencies
* (because npm 6 does not do that but npm 7 does) is not enough. For some
* reason why must, by hand, update the package.json file first and then
* run a normal "npm install".
*/
const fs = require('fs');
const path = require('path');

const version = process.argv[2];
if (!version) {
console.log('Please pass the webpack version - "^4" or "^5" - as an argument.');

process.exit(1);
}

const packageData = JSON.parse(fs.readFileSync(path.join(__dirname, 'package.json')));
packageData.peerDependencies.webpack = version;

packageData.devDependencies.webpack = version;

fs.writeFileSync(path.join(__dirname, 'package.json'), JSON.stringify(packageData, null, 2));
1 change: 1 addition & 0 deletions test/fixtures/import_image.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '../../assets/manifest.svg';
8 changes: 4 additions & 4 deletions test/integration/location.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ test('output to the correct location', async (t) => {
filename: '[name].js',
path: outputPath
},
plugins: [new WebpackManifestPlugin({ fileName: 'webpack.manifest.js' })]
plugins: [new WebpackManifestPlugin({ fileName: 'webpack.manifest.json' })]
};

await compile(config, {}, t);

const manifestPath = join(outputPath, 'webpack.manifest.js');
const manifestPath = join(outputPath, 'webpack.manifest.json');
const result = readJson(manifestPath);

t.deepEqual(result, { 'main.js': 'main.js' });
Expand All @@ -41,11 +41,11 @@ test('output using absolute path', async (t) => {
filename: '[name].js',
path: absOutputPath
},
plugins: [new WebpackManifestPlugin({ fileName: join(absOutputPath, 'webpack.manifest.js') })]
plugins: [new WebpackManifestPlugin({ fileName: join(absOutputPath, 'webpack.manifest.json') })]
};
await compile(config, {}, t);

const manifestPath = join(absOutputPath, 'webpack.manifest.js');
const manifestPath = join(absOutputPath, 'webpack.manifest.json');
const result = readJson(manifestPath);

t.deepEqual(result, { 'main.js': 'main.js' });
Expand Down
45 changes: 37 additions & 8 deletions test/unit/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,15 @@ test('works with source maps', async (t) => {
one: '../fixtures/file.js'
},
output: {
filename: '[name].js',
filename: 'build/[name].js',
path: join(outputPath, 'source-maps')
}
};
const { manifest } = await compile(config, t);

t.deepEqual(manifest, {
'one.js': 'one.js',
'one.js.map': 'one.js.map'
'one.js': 'build/one.js',
'one.js.map': 'build/one.js.map'
});
});

Expand Down Expand Up @@ -158,11 +158,6 @@ test('outputs a manifest of no-js file', async (t) => {
'file.txt': 'file.txt'
};

// Note: I believe this to be another bug in webpack v5 and cannot find a good workaround atm
if (webpack.version.startsWith('5')) {
expected['main.txt'] = 'file.txt';
}

t.truthy(manifest);
t.deepEqual(manifest, expected);
});
Expand All @@ -188,3 +183,37 @@ test('make manifest available to other webpack plugins', async (t) => {
t.pass();
}
});

if (!webpack.version.startsWith('4')) {
test('works with asset modules', async (t) => {
const config = {
context: __dirname,
entry: '../fixtures/import_image.js',
output: {
path: join(outputPath, 'auxiliary-assets'),
assetModuleFilename: `images/[name].[hash:4][ext]`
},
module: {
rules: [
{
test: /\.(svg)/,
type: 'asset/resource'
}
]
}
};

const { manifest } = await compile(config, t);
const expected = {
'main.js': 'main.js',
'images/manifest.svg': `images/manifest.14ca.svg`
};

t.truthy(manifest);
t.deepEqual(Object.keys(expected), ['main.js', 'images/manifest.svg']);
t.deepEqual(manifest['main.js'], 'main.js');
t.regex(manifest['images/manifest.svg'], /images\/manifest\.[a-z|\d]{4}\.svg/);
});
} else {
test.skip('works with asset modules', () => {});
}
4 changes: 2 additions & 2 deletions test/unit/manifest-location.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ test('relative path', async (t) => {
};

const { manifest } = await compile(config, t, {
fileName: 'webpack.manifest.js'
fileName: 'webpack.manifest.json'
});

t.deepEqual(manifest, { 'main.js': 'main.js' });
Expand All @@ -31,7 +31,7 @@ test('absolute path', async (t) => {
};

const { manifest } = await compile(config, t, {
fileName: join(outputPath, 'absolute/webpack.manifest.js')
fileName: join(outputPath, 'absolute/webpack.manifest.json')
});

t.deepEqual(manifest, { 'main.js': 'main.js' });
Expand Down
6 changes: 0 additions & 6 deletions test/unit/paths.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const { join } = require('path');

const test = require('ava');
const del = require('del');
const webpack = require('webpack');

const { compile, hashLiteral } = require('../helpers/unit');

Expand Down Expand Up @@ -200,11 +199,6 @@ test('ensures the manifest is mapping paths to names', async (t) => {
'file.txt': 'outputfile.txt'
};

// Note: I believe this to be another bug in webpack v5 and cannot find a good workaround atm
if (webpack.version.startsWith('5')) {
expected['main.txt'] = 'outputfile.txt';
}

t.truthy(manifest);
t.deepEqual(manifest, expected);
});
Expand Down

0 comments on commit ef1f875

Please sign in to comment.