Skip to content

Commit 01ad7a7

Browse files
weaverryanweaverryan
committed
Fixing Windows support:
* Refactoring delicate path logic into path-util * Fixing logic that relied in unix-style paths * Fixed a bug inside webpack-manifest-plugin that used \ for manifest.json key paths on Windows
1 parent 83341db commit 01ad7a7

File tree

8 files changed

+271
-119
lines changed

8 files changed

+271
-119
lines changed

‎lib/config-generator.js

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const missingLoaderTransformer = require('./friendly-errors/transformers/missing
2222
const missingLoaderFormatter = require('./friendly-errors/formatters/missing-loader');
2323
const missingPostCssConfigTransformer = require('./friendly-errors/transformers/missing-postcss-config');
2424
const missingPostCssConfigFormatter = require('./friendly-errors/formatters/missing-postcss-config');
25+
const pathUtil = require('./config/path-util');
2526

2627
class ConfigGenerator {
2728
/**
@@ -275,8 +276,7 @@ class ConfigGenerator {
275276
*/
276277
let manifestPrefix = this.webpackConfig.manifestKeyPrefix;
277278
if (null === manifestPrefix) {
278-
// by convention, we remove the opening slash on the manifest keys
279-
manifestPrefix = this.webpackConfig.publicPath.replace(/^\//,'');
279+
manifestPrefix = pathUtil.generateManifestKeyPrefix(this.webpackConfig);
280280
}
281281
plugins.push(new ManifestPlugin({
282282
basePath: manifestPrefix,
@@ -407,7 +407,7 @@ class ConfigGenerator {
407407
plugins.push(friendlyErrorsPlugin);
408408

409409
if (!this.webpackConfig.useDevServer()) {
410-
const outputPath = this.webpackConfig.outputPath.replace(this.webpackConfig.getContext() + '/', '');
410+
const outputPath = pathUtil.getRelativeOutputPath(this.webpackConfig);
411411
plugins.push(new AssetOutputDisplayPlugin(outputPath, friendlyErrorsPlugin));
412412
}
413413

@@ -436,24 +436,7 @@ class ConfigGenerator {
436436
}
437437

438438
buildDevServerConfig() {
439-
// strip trailing slash
440-
const outputPath = this.webpackConfig.outputPath.replace(/\/$/,'');
441-
// use the manifestKeyPrefix if available
442-
const publicPath = this.webpackConfig.manifestKeyPrefix ? this.webpackConfig.manifestKeyPrefix.replace(/\/$/,'') : this.webpackConfig.publicPath.replace(/\/$/,'');
443-
444-
/*
445-
* We use the intersection of the publicPath and outputPath to determine
446-
* "document root" of the web server. For example:
447-
* * outputPath = /var/www/public/build
448-
* * publicPath = /build/
449-
* => contentBase should be /var/www/public
450-
*/
451-
if (outputPath.indexOf(publicPath) === -1) {
452-
throw new Error(`Unable to determine contentBase option for webpack's devServer configuration. The publicPath (${this.webpackConfig.publicPath}) string does not exist in the outputPath (${this.webpackConfig.outputPath}), and so the "document root" cannot be determined.`);
453-
}
454-
455-
// a non-regex replace
456-
const contentBase = outputPath.split(publicPath).join('');
439+
const contentBase = pathUtil.getContentBase(this.webpackConfig);
457440

458441
return {
459442
contentBase: contentBase,

‎lib/config/path-util.js

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
* This file is part of the Symfony package.
3+
*
4+
* (c) Fabien Potencier <fabien@symfony.com>
5+
*
6+
* For the full copyright and license information, please view the LICENSE
7+
* file that was distributed with this source code.
8+
*/
9+
10+
'use strict';
11+
12+
const path = require('path');
13+
14+
module.exports = {
15+
/**
16+
* Determines the "contentBase" used for the devServer.
17+
*
18+
* @param {WebpackConfig} webpackConfig
19+
* @return {String}
20+
*/
21+
getContentBase(webpackConfig) {
22+
// strip trailing slash (for Unix or Windows)
23+
const outputPath = webpackConfig.outputPath.replace(/\/$/,'').replace(/\\$/, '');
24+
// use the manifestKeyPrefix if available
25+
const publicPath = webpackConfig.manifestKeyPrefix ? webpackConfig.manifestKeyPrefix.replace(/\/$/,'') : webpackConfig.publicPath.replace(/\/$/,'');
26+
27+
/*
28+
* We use the intersection of the publicPath (or manifestKeyPrefix) and outputPath
29+
* to determine the "document root" of the web server. For example:
30+
* * outputPath = /var/www/public/build
31+
* * publicPath = /build/
32+
* => contentBase should be /var/www/public
33+
*/
34+
35+
// start with outputPath, then join publicPath with it, see if it equals outputPath
36+
// in loop, do dirname on outputPath and repeat
37+
// eventually, you (may) get to the right path
38+
let contentBase = outputPath;
39+
while (path.dirname(contentBase) !== contentBase) {
40+
contentBase = path.dirname(contentBase);
41+
42+
if (path.join(contentBase, publicPath) === outputPath) {
43+
return contentBase;
44+
}
45+
}
46+
47+
throw new Error(`Unable to determine contentBase option for webpack's devServer configuration. The publicPath (${webpackConfig.publicPath}) string does not exist in the outputPath (${webpackConfig.outputPath}), and so the "document root" cannot be determined.`);
48+
},
49+
50+
/**
51+
* Returns the output path, but as a relative string (e.g. web/build)
52+
*
53+
* @param {WebpackConfig} webpackConfig
54+
* @return {String}
55+
*/
56+
getRelativeOutputPath(webpackConfig) {
57+
return webpackConfig.outputPath.replace(webpackConfig.getContext() + '/', '');
58+
},
59+
60+
/**
61+
* If the manifestKeyPrefix is not set, this uses the publicPath to generate it.
62+
*
63+
* Most importantly, this runs some sanity checks to make sure that it's
64+
* ok to use the publicPath as the manifestKeyPrefix.
65+
*
66+
* @param {WebpackConfig} webpackConfig
67+
* @return {String}
68+
*/
69+
generateManifestKeyPrefix(webpackConfig) {
70+
if (webpackConfig.publicPath.includes('://')) {
71+
/*
72+
* If publicPath is absolute, you probably don't want your manifests.json
73+
* keys to be prefixed with the CDN URL. Instead, we force you to
74+
* choose your manifestKeyPrefix.
75+
*/
76+
77+
throw new Error('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use when building your manifest keys. This is happening because you passed an absolute URL to setPublicPath().');
78+
}
79+
80+
let outputPath = webpackConfig.outputPath;
81+
// for comparison purposes, change \ to / on Windows
82+
outputPath = outputPath.split('\\').join('/');
83+
84+
// remove trailing slash on each
85+
outputPath = outputPath.replace(/\/$/, '');
86+
const publicPath = webpackConfig.publicPath.replace(/\/$/, '');
87+
88+
/*
89+
* This is a sanity check. If, for example, you are deploying
90+
* to a subdirectory, then you might have something like this:
91+
* outputPath = /var/www/public/build
92+
* publicPath = /subdir/build/
93+
*
94+
* In that case, you probably don't want the keys in the manifest.json
95+
* file to be prefixed with /subdir/build - it makes more sense
96+
* to prefix them with /build, which is the true prefix relative
97+
* to your application (the subdirectory is a deployment detail).
98+
*
99+
* For that reason, we force you to choose your manifestKeyPrefix().
100+
*/
101+
if (outputPath.indexOf(publicPath) === -1) {
102+
const suggestion = publicPath.substr(publicPath.lastIndexOf('/') + 1) + '/';
103+
104+
throw new Error(`Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. ${suggestion}) to use when building your manifest keys. This is caused by setOutputPath() (${outputPath}) and setPublicPath() (${publicPath}) containing paths that don't seem compatible.`);
105+
}
106+
107+
// by convention, we remove the opening slash on the manifest keys
108+
return webpackConfig.publicPath.replace(/^\//,'');
109+
}
110+
};

‎lib/config/validator.js

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ class Validator {
2020
validate() {
2121
this._validateBasic();
2222

23-
this._validatePublicPathConfig();
24-
2523
this._validateDevServer();
2624
}
2725

@@ -39,42 +37,6 @@ class Validator {
3937
}
4038
}
4139

42-
_validatePublicPathConfig() {
43-
if (this.webpackConfig.publicPath.includes('://') && !this.webpackConfig.manifestKeyPrefix) {
44-
/*
45-
* If publicPath is absolute, you probably don't want your manifests.json
46-
* keys to be prefixed with the CDN URL. Instead, we force you to
47-
* choose your manifestKeyPrefix.
48-
*/
49-
50-
throw new Error('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use when building your manifest keys. This is happening because you passed an absolute URL to setPublicPath().');
51-
}
52-
53-
if (!this.webpackConfig.manifestKeyPrefix) {
54-
const outputPath = this.webpackConfig.outputPath.replace(/\/$/, '');
55-
const publicPath = this.webpackConfig.publicPath.replace(/\/$/, '');
56-
57-
/*
58-
* This is a sanity check. If, for example, you are deploying
59-
* to a subdirectory, then you might have something like this:
60-
* outputPath = /var/www/public/build
61-
* publicPath = /subdir/build/
62-
*
63-
* In that case, you probably don't want the keys in the manifest.json
64-
* file to be prefixed with /subdir/build - it makes more sense
65-
* to prefix them with /build, which is the true prefix relative
66-
* to your application (the subdirectory is a deployment detail).
67-
*
68-
* For that reason, we force you to choose your manifestKeyPrefix().
69-
*/
70-
if (outputPath.indexOf(publicPath) === -1) {
71-
const suggestion = publicPath.substr(publicPath.lastIndexOf('/') + 1) + '/';
72-
73-
throw new Error(`Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. ${suggestion}) to use when building your manifest keys. This is caused by setOutputPath() and setPublicPath() containing paths that don't seem compatible.`);
74-
}
75-
}
76-
}
77-
7840
_validateDevServer() {
7941
if (this.webpackConfig.useVersioning && this.webpackConfig.useDevServer()) {
8042
throw new Error('Don\'t enable versioning with the dev-server. A good setting is Encore.enableVersioning(Encore.isProduction()).');

‎lib/webpack/webpack-manifest-plugin.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ ManifestPlugin.prototype.apply = function(compiler) {
4545
path.dirname(file),
4646
path.basename(module.userRequest)
4747
);
48+
/* *** MODIFICATION START *** */
49+
// for Windows, we want the keys to use /, not \
50+
// (and path.join will obviously use \ in Windows)
51+
if (process.platform === 'win32') {
52+
moduleAssets[file] = moduleAssets[file].split('\\').join('/');
53+
}
54+
/* *** MODIFICATION END *** */
4855
});
4956
});
5057

‎test/config-generator.js

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -466,38 +466,5 @@ describe('The config-generator function', () => {
466466
const actualConfig = configGenerator(config);
467467
expect(actualConfig.devServer).to.be.undefined;
468468
});
469-
470-
it('contentBase is calculated correctly', () => {
471-
const config = createConfig();
472-
config.runtimeConfig.useDevServer = true;
473-
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
474-
config.outputPath = '/tmp/public/build';
475-
config.setPublicPath('/build/');
476-
config.addEntry('main', './main');
477-
478-
const actualConfig = configGenerator(config);
479-
// contentBase should point to the "document root", which
480-
// is calculated as outputPath, but without the publicPath portion
481-
expect(actualConfig.devServer.contentBase).to.equal('/tmp/public');
482-
expect(actualConfig.devServer.publicPath).to.equal('/build/');
483-
484-
});
485-
486-
it('contentBase works ok with manifestKeyPrefix', () => {
487-
const config = createConfig();
488-
config.runtimeConfig.useDevServer = true;
489-
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
490-
config.outputPath = '/tmp/public/build';
491-
config.setPublicPath('/subdirectory/build');
492-
// this "fixes" the incompatibility between outputPath and publicPath
493-
config.setManifestKeyPrefix('/build/');
494-
config.addEntry('main', './main');
495-
496-
const actualConfig = configGenerator(config);
497-
// contentBase should point to the "document root", which
498-
// is calculated as outputPath, but without the publicPath portion
499-
expect(actualConfig.devServer.contentBase).to.equal('/tmp/public');
500-
expect(actualConfig.devServer.publicPath).to.equal('/subdirectory/build/');
501-
});
502469
});
503470
});

‎test/config/path-util.js

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
/*
2+
* This file is part of the Symfony package.
3+
*
4+
* (c) Fabien Potencier <fabien@symfony.com>
5+
*
6+
* For the full copyright and license information, please view the LICENSE
7+
* file that was distributed with this source code.
8+
*/
9+
10+
'use strict';
11+
12+
const expect = require('chai').expect;
13+
const WebpackConfig = require('../../lib/WebpackConfig');
14+
const RuntimeConfig = require('../../lib/config/RuntimeConfig');
15+
const pathUtil = require('../../lib/config/path-util');
16+
const process = require('process');
17+
18+
function createConfig() {
19+
const runtimeConfig = new RuntimeConfig();
20+
runtimeConfig.context = __dirname;
21+
runtimeConfig.environment = 'dev';
22+
runtimeConfig.babelRcFileExists = false;
23+
24+
return new WebpackConfig(runtimeConfig);
25+
}
26+
27+
/**
28+
* Some tests are very specific to different operating systems.
29+
* We use this to only run them when needed.
30+
*
31+
* @returns {boolean}
32+
*/
33+
function isWindows() {
34+
return process.platform === 'win32';
35+
}
36+
37+
describe('path-util getContentBase()', () => {
38+
describe('getContentBase()', () => {
39+
it('contentBase is calculated correctly', function() {
40+
if (isWindows()) {
41+
this.skip();
42+
}
43+
44+
const config = createConfig();
45+
config.runtimeConfig.useDevServer = true;
46+
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
47+
config.outputPath = '/tmp/public/build';
48+
config.setPublicPath('/build/');
49+
config.addEntry('main', './main');
50+
51+
const actualContentBase = pathUtil.getContentBase(config);
52+
// contentBase should point to the "document root", which
53+
// is calculated as outputPath, but without the publicPath portion
54+
expect(actualContentBase).to.equal('/tmp/public');
55+
});
56+
57+
it('contentBase works ok with manifestKeyPrefix', function() {
58+
if (isWindows()) {
59+
this.skip();
60+
}
61+
62+
const config = createConfig();
63+
config.runtimeConfig.useDevServer = true;
64+
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
65+
config.outputPath = '/tmp/public/build';
66+
config.setPublicPath('/subdirectory/build');
67+
// this "fixes" the incompatibility between outputPath and publicPath
68+
config.setManifestKeyPrefix('/build/');
69+
config.addEntry('main', './main');
70+
71+
const actualContentBase = pathUtil.getContentBase(config);
72+
expect(actualContentBase).to.equal('/tmp/public');
73+
});
74+
75+
it('contentBase is calculated correctly on Windows', function() {
76+
if (!isWindows()) {
77+
this.skip();
78+
}
79+
80+
const config = createConfig();
81+
config.runtimeConfig.useDevServer = true;
82+
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
83+
config.outputPath = 'C:\\projects\\webpack-encore\\public\\build';
84+
config.setPublicPath('/build/');
85+
config.addEntry('main', './main');
86+
87+
const actualContentBase = pathUtil.getContentBase(config);
88+
expect(actualContentBase).to.equal('C:\\projects\\webpack-encore\\public');
89+
});
90+
});
91+
92+
describe('generateManifestKeyPrefix', () => {
93+
it('manifestKeyPrefix is correctly not required on windows', () => {
94+
const config = createConfig();
95+
config.outputPath = 'C:\\projects\\webpack-encore\\web\\build';
96+
config.setPublicPath('/build/');
97+
config.addEntry('main', './main');
98+
99+
const actualPrefix = pathUtil.generateManifestKeyPrefix(config);
100+
expect(actualPrefix).to.equal('build/');
101+
});
102+
103+
it('with absolute publicPath, manifestKeyPrefix must be set', () => {
104+
const config = createConfig();
105+
config.outputPath = '/tmp/public/build';
106+
config.setPublicPath('/build');
107+
config.addEntry('main', './main');
108+
config.setPublicPath('https://cdn.example.com');
109+
110+
expect(() => {
111+
pathUtil.generateManifestKeyPrefix(config);
112+
}).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use');
113+
});
114+
115+
it('when outputPath and publicPath are incompatible, manifestKeyPrefix must be set', () => {
116+
const config = createConfig();
117+
config.outputPath = '/tmp/public/build';
118+
config.addEntry('main', './main');
119+
// pretend we're installed to a subdirectory
120+
config.setPublicPath('/subdirectory/build');
121+
122+
expect(() => {
123+
pathUtil.generateManifestKeyPrefix(config);
124+
}).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use');
125+
});
126+
});
127+
});

0 commit comments

Comments
 (0)