Skip to content

Commit 71eea68

Browse files
weaverryanweaverryan
committed
bug #28 Fixing Windows Support (weaverryan)
This PR was squashed before being merged into the master branch (closes #28). Discussion ---------- Fixing Windows Support In a few places, we rely (relied) on doing some "string replace" on paths. Obviously, that doesn't play well with the windows `\` directory separators. This fixes the few places that rely on this and includes tests that exposed those failures on AppVeyor. Commits ------- 78a0680 cs 6164d80 Tweaks thanks to Stof, including combining Unix & windows tests 162d5f5 Moving some validation logic to be more clear about the flow 5f8ff97 Fixing a small bug when reporting the relative output path 6c65840 Fixing Windows support:
2 parents d80d21b + 78a0680 commit 71eea68

File tree

8 files changed

+267
-115
lines changed

8 files changed

+267
-115
lines changed

‎lib/config-generator.js

Lines changed: 3 additions & 19 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
/**
@@ -407,7 +408,7 @@ class ConfigGenerator {
407408
plugins.push(friendlyErrorsPlugin);
408409

409410
if (!this.webpackConfig.useDevServer()) {
410-
const outputPath = this.webpackConfig.outputPath.replace(this.webpackConfig.getContext() + '/', '');
411+
const outputPath = pathUtil.getRelativeOutputPath(this.webpackConfig);
411412
plugins.push(new AssetOutputDisplayPlugin(outputPath, friendlyErrorsPlugin));
412413
}
413414

@@ -440,24 +441,7 @@ class ConfigGenerator {
440441
}
441442

442443
buildDevServerConfig() {
443-
// strip trailing slash
444-
const outputPath = this.webpackConfig.outputPath.replace(/\/$/,'');
445-
// use the manifestKeyPrefix if available
446-
const publicPath = this.webpackConfig.manifestKeyPrefix ? this.webpackConfig.manifestKeyPrefix.replace(/\/$/,'') : this.webpackConfig.publicPath.replace(/\/$/,'');
447-
448-
/*
449-
* We use the intersection of the publicPath and outputPath to determine
450-
* "document root" of the web server. For example:
451-
* * outputPath = /var/www/public/build
452-
* * publicPath = /build/
453-
* => contentBase should be /var/www/public
454-
*/
455-
if (outputPath.indexOf(publicPath) === -1) {
456-
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.`);
457-
}
458-
459-
// a non-regex replace
460-
const contentBase = outputPath.split(publicPath).join('');
444+
const contentBase = pathUtil.getContentBase(this.webpackConfig);
461445

462446
return {
463447
contentBase: contentBase,

‎lib/config/path-util.js

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* This file is part of the Symfony package.
3+
*
4+
* (c) Fabien Potencier <[email protected]>
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" to use 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+
* At this point, if the publicPath is non-standard (e.g. it contains
35+
* a sub-directory or is absolute), then the user will already see
36+
* an error that they must set the manifestKeyPrefix.
37+
*/
38+
39+
// start with outputPath, then join publicPath with it, see if it equals outputPath
40+
// in loop, do dirname on outputPath and repeat
41+
// eventually, you (may) get to the right path
42+
let contentBase = outputPath;
43+
while (path.dirname(contentBase) !== contentBase) {
44+
contentBase = path.dirname(contentBase);
45+
46+
if (path.join(contentBase, publicPath) === outputPath) {
47+
return contentBase;
48+
}
49+
}
50+
51+
throw new Error(`Unable to determine contentBase option for webpack's devServer configuration. The ${webpackConfig.manifestKeyPrefix ? 'manifestKeyPrefix' : 'publicPath'} (${webpackConfig.manifestKeyPrefix ? webpackConfig.manifestKeyPrefix : webpackConfig.publicPath}) string does not exist in the outputPath (${webpackConfig.outputPath}), and so the "document root" cannot be determined.`);
52+
},
53+
54+
/**
55+
* Returns the output path, but as a relative string (e.g. web/build)
56+
*
57+
* @param {WebpackConfig} webpackConfig
58+
* @return {String}
59+
*/
60+
getRelativeOutputPath(webpackConfig) {
61+
return webpackConfig.outputPath.replace(webpackConfig.getContext() + path.sep, '');
62+
},
63+
64+
/**
65+
* If the manifestKeyPrefix is not set, this uses the publicPath to generate it.
66+
*
67+
* Most importantly, this runs some sanity checks to make sure that it's
68+
* ok to use the publicPath as the manifestKeyPrefix.
69+
*
70+
* @param {WebpackConfig} webpackConfig
71+
* @return {void}
72+
*/
73+
validatePublicPathAndManifestKeyPrefix(webpackConfig) {
74+
if (webpackConfig.manifestKeyPrefix !== null) {
75+
// nothing to check - they have manually set the key prefix
76+
return;
77+
}
78+
79+
if (webpackConfig.publicPath.includes('://')) {
80+
/*
81+
* If publicPath is absolute, you probably don't want your manifests.json
82+
* keys to be prefixed with the CDN URL. Instead, we force you to
83+
* choose your manifestKeyPrefix.
84+
*/
85+
86+
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().');
87+
}
88+
89+
let outputPath = webpackConfig.outputPath;
90+
// for comparison purposes, change \ to / on Windows
91+
outputPath = outputPath.replace(/\\/g, '/');
92+
93+
// remove trailing slash on each
94+
outputPath = outputPath.replace(/\/$/, '');
95+
const publicPath = webpackConfig.publicPath.replace(/\/$/, '');
96+
97+
/*
98+
* This is a sanity check. If, for example, you are deploying
99+
* to a subdirectory, then you might have something like this:
100+
* outputPath = /var/www/public/build
101+
* publicPath = /subdir/build/
102+
*
103+
* In that case, you probably don't want the keys in the manifest.json
104+
* file to be prefixed with /subdir/build - it makes more sense
105+
* to prefix them with /build, which is the true prefix relative
106+
* to your application (the subdirectory is a deployment detail).
107+
*
108+
* For that reason, we force you to choose your manifestKeyPrefix().
109+
*/
110+
if (outputPath.indexOf(publicPath) === -1) {
111+
const suggestion = publicPath.substr(publicPath.lastIndexOf('/') + 1) + '/';
112+
113+
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.`);
114+
}
115+
}
116+
};

‎lib/config/validator.js

Lines changed: 5 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
'use strict';
1111

12+
const pathUtil = require('./path-util');
13+
1214
class Validator {
1315
/**
1416
* @param {WebpackConfig} webpackConfig
@@ -20,7 +22,7 @@ class Validator {
2022
validate() {
2123
this._validateBasic();
2224

23-
this._validatePublicPathConfig();
25+
this._validatePublicPathAndManifestKeyPrefix();
2426

2527
this._validateDevServer();
2628
}
@@ -39,40 +41,8 @@ class Validator {
3941
}
4042
}
4143

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-
}
44+
_validatePublicPathAndManifestKeyPrefix() {
45+
pathUtil.validatePublicPathAndManifestKeyPrefix(this.webpackConfig);
7646
}
7747

7848
_validateDevServer() {

‎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].replace(/\\/g, '/');
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,39 +466,6 @@ 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

504471
describe('test for addPlugin config', () => {

‎test/config/path-util.js

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* This file is part of the Symfony package.
3+
*
4+
* (c) Fabien Potencier <[email protected]>
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+
const isWindows = (process.platform === 'win32');
28+
29+
describe('path-util getContentBase()', () => {
30+
describe('getContentBase()', () => {
31+
it('contentBase is calculated correctly', function() {
32+
const config = createConfig();
33+
config.runtimeConfig.useDevServer = true;
34+
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
35+
config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build';
36+
config.setPublicPath('/build/');
37+
config.addEntry('main', './main');
38+
39+
const actualContentBase = pathUtil.getContentBase(config);
40+
// contentBase should point to the "document root", which
41+
// is calculated as outputPath, but without the publicPath portion
42+
expect(actualContentBase).to.equal(isWindows ? 'C:\\tmp\\public' : '/tmp/public');
43+
});
44+
45+
it('contentBase works ok with manifestKeyPrefix', function() {
46+
const config = createConfig();
47+
config.runtimeConfig.useDevServer = true;
48+
config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
49+
config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build';
50+
config.setPublicPath('/subdirectory/build');
51+
// this "fixes" the incompatibility between outputPath and publicPath
52+
config.setManifestKeyPrefix('/build/');
53+
config.addEntry('main', './main');
54+
55+
const actualContentBase = pathUtil.getContentBase(config);
56+
expect(actualContentBase).to.equal(isWindows ? 'C:\\tmp\\public' : '/tmp/public');
57+
});
58+
});
59+
60+
describe('validatePublicPathAndManifestKeyPrefix', () => {
61+
it('manifestKeyPrefix is correctly not required on windows', () => {
62+
const config = createConfig();
63+
config.outputPath = 'C:\\projects\\webpack-encore\\web\\build';
64+
config.setPublicPath('/build/');
65+
config.addEntry('main', './main');
66+
67+
// NOT throwing an error is the assertion
68+
pathUtil.validatePublicPathAndManifestKeyPrefix(config);
69+
});
70+
71+
it('with absolute publicPath, manifestKeyPrefix must be set', () => {
72+
const config = createConfig();
73+
config.outputPath = '/tmp/public/build';
74+
config.setPublicPath('/build');
75+
config.addEntry('main', './main');
76+
config.setPublicPath('https://cdn.example.com');
77+
78+
expect(() => {
79+
pathUtil.validatePublicPathAndManifestKeyPrefix(config);
80+
}).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use');
81+
});
82+
83+
it('when outputPath and publicPath are incompatible, manifestKeyPrefix must be set', () => {
84+
const config = createConfig();
85+
86+
config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build';
87+
config.addEntry('main', './main');
88+
// pretend we're installed to a subdirectory
89+
config.setPublicPath('/subdirectory/build');
90+
91+
expect(() => {
92+
pathUtil.validatePublicPathAndManifestKeyPrefix(config);
93+
}).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use');
94+
});
95+
});
96+
97+
describe('getRelativeOutputPath', () => {
98+
it('basic usage', function() {
99+
const config = createConfig();
100+
if (isWindows) {
101+
config.runtimeConfig.context = 'C:\\projects\\webpack-encore';
102+
config.outputPath = 'C:\\projects\\webpack-encore\\public\\build';
103+
} else {
104+
config.runtimeConfig.context = '/tmp/webpack-encore';
105+
config.outputPath = '/tmp/webpack-encore/public/build';
106+
}
107+
108+
const actualPath = pathUtil.getRelativeOutputPath(config);
109+
expect(actualPath).to.equal(isWindows ? 'public\\build' : 'public/build');
110+
});
111+
});
112+
});

0 commit comments

Comments
 (0)