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.

Sign up for GitHub

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

Fix the exception thrown when writeToDisk is enabled in multi-compiler mode #301

Merged
merged 4 commits into from
Apr 19, 2018
Merged

Fix the exception thrown when writeToDisk is enabled in multi-compiler mode #301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e03496
fix writeToDisk option in multi-compiler
arianrhodsandlot Apr 19, 2018
78ee291
rmdir -> rmdirSync
arianrhodsandlot Apr 19, 2018
73ffb5d
use a better solution of toDisk for multi-compilers
arianrhodsandlot Apr 19, 2018
694389c
use a shorten method name
arianrhodsandlot Apr 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 31 additions & 28 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,42 +12,45 @@ const { mkdirp } = new NodeOutputFileSystem();

module.exports = {
toDisk(context) {
context.compiler.hooks.afterEmit.tap('WebpackDevMiddleware', (compilation) => {
const { assets, compiler } = compilation;
const { log } = context;
const { writeToDisk: filter } = context.options;
let { outputPath } = compiler;
const compilers = context.compiler.compilers || [context.compiler];
for (const compiler of compilers) {
compiler.hooks.afterEmit.tap('WebpackDevMiddleware', (compilation) => {
const { assets } = compilation;
const { log } = context;
const { writeToDisk: filter } = context.options;
let { outputPath } = compiler;

if (outputPath === '/') {
outputPath = compiler.context;
}
if (outputPath === '/') {
outputPath = compiler.context;
}

for (const assetPath of Object.keys(assets)) {
const asset = assets[assetPath];
const source = asset.source();
const isAbsolute = pathabs(assetPath);
const writePath = isAbsolute ? assetPath : path.join(outputPath, assetPath);
const relativePath = path.relative(process.cwd(), writePath);
const allowWrite = filter && typeof filter === 'function' ? filter(writePath) : true;
for (const assetPath of Object.keys(assets)) {
const asset = assets[assetPath];
const source = asset.source();
const isAbsolute = pathabs(assetPath);
const writePath = isAbsolute ? assetPath : path.join(outputPath, assetPath);
const relativePath = path.relative(process.cwd(), writePath);
const allowWrite = filter && typeof filter === 'function' ? filter(writePath) : true;

if (allowWrite) {
let output = source;
if (allowWrite) {
let output = source;

mkdirp.sync(path.dirname(writePath));
mkdirp.sync(path.dirname(writePath));

if (Array.isArray(source)) {
output = source.join('\n');
}
if (Array.isArray(source)) {
output = source.join('\n');
}

try {
fs.writeFileSync(writePath, output, 'utf-8');
log.debug(chalk`{cyan Asset written to disk}: ${relativePath}`);
} catch (e) {
log.error(`Unable to write asset to disk:\n${e}`);
try {
fs.writeFileSync(writePath, output, 'utf-8');
log.debug(chalk`{cyan Asset written to disk}: ${relativePath}`);
} catch (e) {
log.error(`Unable to write asset to disk:\n${e}`);
}
}
}
}
});
});
}
},

setFs(context, compiler) {
Expand Down
6 changes: 4 additions & 2 deletions test/fixtures/server-test/webpack.array.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
'use strict';

const path = require('path');

module.exports = [{
mode: 'development',
context: __dirname,
entry: './foo.js',
output: {
filename: 'foo.js',
path: '/js1',
path: path.resolve(__dirname, 'js1'),
Copy link
Contributor

Choose a reason for hiding this comment

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

curious: previous tests pass on this with the path.resolve. is there a reason for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As documented, output.path should be an absolute path. And without path.resolve, it will attempt to write files into the root directory /. This configuration didn't cause an error in the past because files are all in memory when no test cases with writeToDisk: trueare using it.

Copy link
Contributor

Choose a reason for hiding this comment

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

excellent, thank you

publicPath: '/js1/'
},
module: {
Expand All @@ -24,7 +26,7 @@ module.exports = [{
entry: './bar.js',
output: {
filename: 'bar.js',
path: '/js2',
path: path.resolve(__dirname, 'js2'),
publicPath: '/js2/'
}
}];
45 changes: 45 additions & 0 deletions test/tests/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,4 +377,49 @@ describe('Server', () => {
});
});
});

function multiToDisk(value, done) {
app = express();
const compiler = webpack(webpackMultiConfig);
instance = middleware(compiler, {
stats: 'errors-only',
logLevel,
writeToDisk: value
});
app.use(instance);
app.use((req, res) => {
res.sendStatus(200);
});
listen = listenShorthand(done);
}

describe('write to disk with MultiCompiler', () => {
before((done) => {
multiToDisk(true, done);
});
after(close);

it('should find the bundle files on disk', (done) => {
request(app).get('/foo/bar')
.expect(200, () => {
const bundleFiles = [
'../fixtures/server-test/js1/foo.js',
'../fixtures/server-test/js1/index.html',
'../fixtures/server-test/js1/svg.svg',
'../fixtures/server-test/js2/bar.js'
];

for (const bundleFile of bundleFiles) {
const bundlePath = path.join(__dirname, bundleFile);
assert(fs.existsSync(bundlePath));
fs.unlinkSync(bundlePath);
}

fs.rmdirSync(path.join(__dirname, '../fixtures/server-test/js1/'));
fs.rmdirSync(path.join(__dirname, '../fixtures/server-test/js2/'));

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