diff --git a/lib/config-generator.js b/lib/config-generator.js index 8467c796..450d1e04 100644 --- a/lib/config-generator.js +++ b/lib/config-generator.js @@ -22,6 +22,7 @@ const missingLoaderTransformer = require('./friendly-errors/transformers/missing const missingLoaderFormatter = require('./friendly-errors/formatters/missing-loader'); const missingPostCssConfigTransformer = require('./friendly-errors/transformers/missing-postcss-config'); const missingPostCssConfigFormatter = require('./friendly-errors/formatters/missing-postcss-config'); +const pathUtil = require('./config/path-util'); class ConfigGenerator { /** @@ -407,7 +408,7 @@ class ConfigGenerator { plugins.push(friendlyErrorsPlugin); if (!this.webpackConfig.useDevServer()) { - const outputPath = this.webpackConfig.outputPath.replace(this.webpackConfig.getContext() + '/', ''); + const outputPath = pathUtil.getRelativeOutputPath(this.webpackConfig); plugins.push(new AssetOutputDisplayPlugin(outputPath, friendlyErrorsPlugin)); } @@ -440,24 +441,7 @@ class ConfigGenerator { } buildDevServerConfig() { - // strip trailing slash - const outputPath = this.webpackConfig.outputPath.replace(/\/$/,''); - // use the manifestKeyPrefix if available - const publicPath = this.webpackConfig.manifestKeyPrefix ? this.webpackConfig.manifestKeyPrefix.replace(/\/$/,'') : this.webpackConfig.publicPath.replace(/\/$/,''); - - /* - * We use the intersection of the publicPath and outputPath to determine - * "document root" of the web server. For example: - * * outputPath = /var/www/public/build - * * publicPath = /build/ - * => contentBase should be /var/www/public - */ - if (outputPath.indexOf(publicPath) === -1) { - 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.`); - } - - // a non-regex replace - const contentBase = outputPath.split(publicPath).join(''); + const contentBase = pathUtil.getContentBase(this.webpackConfig); return { contentBase: contentBase, diff --git a/lib/config/path-util.js b/lib/config/path-util.js new file mode 100644 index 00000000..21380184 --- /dev/null +++ b/lib/config/path-util.js @@ -0,0 +1,116 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +const path = require('path'); + +module.exports = { + /** + * Determines the "contentBase" to use for the devServer. + * + * @param {WebpackConfig} webpackConfig + * @return {String} + */ + getContentBase(webpackConfig) { + // strip trailing slash (for Unix or Windows) + const outputPath = webpackConfig.outputPath.replace(/\/$/,'').replace(/\\$/, ''); + // use the manifestKeyPrefix if available + const publicPath = webpackConfig.manifestKeyPrefix ? webpackConfig.manifestKeyPrefix.replace(/\/$/,'') : webpackConfig.publicPath.replace(/\/$/,''); + + /* + * We use the intersection of the publicPath (or manifestKeyPrefix) and outputPath + * to determine the "document root" of the web server. For example: + * * outputPath = /var/www/public/build + * * publicPath = /build/ + * => contentBase should be /var/www/public + * + * At this point, if the publicPath is non-standard (e.g. it contains + * a sub-directory or is absolute), then the user will already see + * an error that they must set the manifestKeyPrefix. + */ + + // start with outputPath, then join publicPath with it, see if it equals outputPath + // in loop, do dirname on outputPath and repeat + // eventually, you (may) get to the right path + let contentBase = outputPath; + while (path.dirname(contentBase) !== contentBase) { + contentBase = path.dirname(contentBase); + + if (path.join(contentBase, publicPath) === outputPath) { + return contentBase; + } + } + + 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.`); + }, + + /** + * Returns the output path, but as a relative string (e.g. web/build) + * + * @param {WebpackConfig} webpackConfig + * @return {String} + */ + getRelativeOutputPath(webpackConfig) { + return webpackConfig.outputPath.replace(webpackConfig.getContext() + path.sep, ''); + }, + + /** + * If the manifestKeyPrefix is not set, this uses the publicPath to generate it. + * + * Most importantly, this runs some sanity checks to make sure that it's + * ok to use the publicPath as the manifestKeyPrefix. + * + * @param {WebpackConfig} webpackConfig + * @return {void} + */ + validatePublicPathAndManifestKeyPrefix(webpackConfig) { + if (webpackConfig.manifestKeyPrefix !== null) { + // nothing to check - they have manually set the key prefix + return; + } + + if (webpackConfig.publicPath.includes('://')) { + /* + * If publicPath is absolute, you probably don't want your manifests.json + * keys to be prefixed with the CDN URL. Instead, we force you to + * choose your manifestKeyPrefix. + */ + + 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().'); + } + + let outputPath = webpackConfig.outputPath; + // for comparison purposes, change \ to / on Windows + outputPath = outputPath.replace(/\\/g, '/'); + + // remove trailing slash on each + outputPath = outputPath.replace(/\/$/, ''); + const publicPath = webpackConfig.publicPath.replace(/\/$/, ''); + + /* + * This is a sanity check. If, for example, you are deploying + * to a subdirectory, then you might have something like this: + * outputPath = /var/www/public/build + * publicPath = /subdir/build/ + * + * In that case, you probably don't want the keys in the manifest.json + * file to be prefixed with /subdir/build - it makes more sense + * to prefix them with /build, which is the true prefix relative + * to your application (the subdirectory is a deployment detail). + * + * For that reason, we force you to choose your manifestKeyPrefix(). + */ + if (outputPath.indexOf(publicPath) === -1) { + const suggestion = publicPath.substr(publicPath.lastIndexOf('/') + 1) + '/'; + + 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.`); + } + } +}; diff --git a/lib/config/validator.js b/lib/config/validator.js index 7778db1b..14f6952c 100644 --- a/lib/config/validator.js +++ b/lib/config/validator.js @@ -9,6 +9,8 @@ 'use strict'; +const pathUtil = require('./path-util'); + class Validator { /** * @param {WebpackConfig} webpackConfig @@ -20,7 +22,7 @@ class Validator { validate() { this._validateBasic(); - this._validatePublicPathConfig(); + this._validatePublicPathAndManifestKeyPrefix(); this._validateDevServer(); } @@ -39,40 +41,8 @@ class Validator { } } - _validatePublicPathConfig() { - if (this.webpackConfig.publicPath.includes('://') && !this.webpackConfig.manifestKeyPrefix) { - /* - * If publicPath is absolute, you probably don't want your manifests.json - * keys to be prefixed with the CDN URL. Instead, we force you to - * choose your manifestKeyPrefix. - */ - - 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().'); - } - - if (!this.webpackConfig.manifestKeyPrefix) { - const outputPath = this.webpackConfig.outputPath.replace(/\/$/, ''); - const publicPath = this.webpackConfig.publicPath.replace(/\/$/, ''); - - /* - * This is a sanity check. If, for example, you are deploying - * to a subdirectory, then you might have something like this: - * outputPath = /var/www/public/build - * publicPath = /subdir/build/ - * - * In that case, you probably don't want the keys in the manifest.json - * file to be prefixed with /subdir/build - it makes more sense - * to prefix them with /build, which is the true prefix relative - * to your application (the subdirectory is a deployment detail). - * - * For that reason, we force you to choose your manifestKeyPrefix(). - */ - if (outputPath.indexOf(publicPath) === -1) { - const suggestion = publicPath.substr(publicPath.lastIndexOf('/') + 1) + '/'; - - 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.`); - } - } + _validatePublicPathAndManifestKeyPrefix() { + pathUtil.validatePublicPathAndManifestKeyPrefix(this.webpackConfig); } _validateDevServer() { diff --git a/lib/webpack/webpack-manifest-plugin.js b/lib/webpack/webpack-manifest-plugin.js index 49ee37d6..796e774f 100644 --- a/lib/webpack/webpack-manifest-plugin.js +++ b/lib/webpack/webpack-manifest-plugin.js @@ -45,6 +45,13 @@ ManifestPlugin.prototype.apply = function(compiler) { path.dirname(file), path.basename(module.userRequest) ); + /* *** MODIFICATION START *** */ + // for Windows, we want the keys to use /, not \ + // (and path.join will obviously use \ in Windows) + if (process.platform === 'win32') { + moduleAssets[file] = moduleAssets[file].replace(/\\/g, '/'); + } + /* *** MODIFICATION END *** */ }); }); diff --git a/test/config-generator.js b/test/config-generator.js index b8fbbcfe..7d387f72 100644 --- a/test/config-generator.js +++ b/test/config-generator.js @@ -466,39 +466,6 @@ describe('The config-generator function', () => { const actualConfig = configGenerator(config); expect(actualConfig.devServer).to.be.undefined; }); - - it('contentBase is calculated correctly', () => { - const config = createConfig(); - config.runtimeConfig.useDevServer = true; - config.runtimeConfig.devServerUrl = 'http://localhost:8080/'; - config.outputPath = '/tmp/public/build'; - config.setPublicPath('/build/'); - config.addEntry('main', './main'); - - const actualConfig = configGenerator(config); - // contentBase should point to the "document root", which - // is calculated as outputPath, but without the publicPath portion - expect(actualConfig.devServer.contentBase).to.equal('/tmp/public'); - expect(actualConfig.devServer.publicPath).to.equal('/build/'); - - }); - - it('contentBase works ok with manifestKeyPrefix', () => { - const config = createConfig(); - config.runtimeConfig.useDevServer = true; - config.runtimeConfig.devServerUrl = 'http://localhost:8080/'; - config.outputPath = '/tmp/public/build'; - config.setPublicPath('/subdirectory/build'); - // this "fixes" the incompatibility between outputPath and publicPath - config.setManifestKeyPrefix('/build/'); - config.addEntry('main', './main'); - - const actualConfig = configGenerator(config); - // contentBase should point to the "document root", which - // is calculated as outputPath, but without the publicPath portion - expect(actualConfig.devServer.contentBase).to.equal('/tmp/public'); - expect(actualConfig.devServer.publicPath).to.equal('/subdirectory/build/'); - }); }); describe('test for addPlugin config', () => { diff --git a/test/config/path-util.js b/test/config/path-util.js new file mode 100644 index 00000000..779a8c3f --- /dev/null +++ b/test/config/path-util.js @@ -0,0 +1,112 @@ +/* + * This file is part of the Symfony package. + * + * (c) Fabien Potencier + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +'use strict'; + +const expect = require('chai').expect; +const WebpackConfig = require('../../lib/WebpackConfig'); +const RuntimeConfig = require('../../lib/config/RuntimeConfig'); +const pathUtil = require('../../lib/config/path-util'); +const process = require('process'); + +function createConfig() { + const runtimeConfig = new RuntimeConfig(); + runtimeConfig.context = __dirname; + runtimeConfig.environment = 'dev'; + runtimeConfig.babelRcFileExists = false; + + return new WebpackConfig(runtimeConfig); +} + +const isWindows = (process.platform === 'win32'); + +describe('path-util getContentBase()', () => { + describe('getContentBase()', () => { + it('contentBase is calculated correctly', function() { + const config = createConfig(); + config.runtimeConfig.useDevServer = true; + config.runtimeConfig.devServerUrl = 'http://localhost:8080/'; + config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build'; + config.setPublicPath('/build/'); + config.addEntry('main', './main'); + + const actualContentBase = pathUtil.getContentBase(config); + // contentBase should point to the "document root", which + // is calculated as outputPath, but without the publicPath portion + expect(actualContentBase).to.equal(isWindows ? 'C:\\tmp\\public' : '/tmp/public'); + }); + + it('contentBase works ok with manifestKeyPrefix', function() { + const config = createConfig(); + config.runtimeConfig.useDevServer = true; + config.runtimeConfig.devServerUrl = 'http://localhost:8080/'; + config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build'; + config.setPublicPath('/subdirectory/build'); + // this "fixes" the incompatibility between outputPath and publicPath + config.setManifestKeyPrefix('/build/'); + config.addEntry('main', './main'); + + const actualContentBase = pathUtil.getContentBase(config); + expect(actualContentBase).to.equal(isWindows ? 'C:\\tmp\\public' : '/tmp/public'); + }); + }); + + describe('validatePublicPathAndManifestKeyPrefix', () => { + it('manifestKeyPrefix is correctly not required on windows', () => { + const config = createConfig(); + config.outputPath = 'C:\\projects\\webpack-encore\\web\\build'; + config.setPublicPath('/build/'); + config.addEntry('main', './main'); + + // NOT throwing an error is the assertion + pathUtil.validatePublicPathAndManifestKeyPrefix(config); + }); + + it('with absolute publicPath, manifestKeyPrefix must be set', () => { + const config = createConfig(); + config.outputPath = '/tmp/public/build'; + config.setPublicPath('/build'); + config.addEntry('main', './main'); + config.setPublicPath('https://cdn.example.com'); + + expect(() => { + pathUtil.validatePublicPathAndManifestKeyPrefix(config); + }).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use'); + }); + + it('when outputPath and publicPath are incompatible, manifestKeyPrefix must be set', () => { + const config = createConfig(); + + config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build'; + config.addEntry('main', './main'); + // pretend we're installed to a subdirectory + config.setPublicPath('/subdirectory/build'); + + expect(() => { + pathUtil.validatePublicPathAndManifestKeyPrefix(config); + }).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use'); + }); + }); + + describe('getRelativeOutputPath', () => { + it('basic usage', function() { + const config = createConfig(); + if (isWindows) { + config.runtimeConfig.context = 'C:\\projects\\webpack-encore'; + config.outputPath = 'C:\\projects\\webpack-encore\\public\\build'; + } else { + config.runtimeConfig.context = '/tmp/webpack-encore'; + config.outputPath = '/tmp/webpack-encore/public/build'; + } + + const actualPath = pathUtil.getRelativeOutputPath(config); + expect(actualPath).to.equal(isWindows ? 'public\\build' : 'public/build'); + }); + }); +}); diff --git a/test/config/validator.js b/test/config/validator.js index 023e03b2..9ab2810c 100644 --- a/test/config/validator.js +++ b/test/config/validator.js @@ -53,30 +53,6 @@ describe('The validator function', () => { }).to.throw('Missing public path'); }); - it('with absolute publicPath, manifestKeyPrefix must be set', () => { - const config = createConfig(); - config.outputPath = '/tmp/public/build'; - config.setPublicPath('/build'); - config.addEntry('main', './main'); - config.setPublicPath('https://cdn.example.com'); - - expect(() => { - validator(config); - }).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use'); - }); - - it('when outputPath and publicPath are incompatible, manifestKeyPrefix must be set', () => { - const config = createConfig(); - config.outputPath = '/tmp/public/build'; - config.addEntry('main', './main'); - // pretend we're installed to a subdirectory - config.setPublicPath('/subdirectory/build'); - - expect(() => { - validator(config); - }).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use'); - }); - it('cannot use versioning with webpack-dev-server', () => { const config = createConfig(); config.outputPath = '/tmp/public/build'; diff --git a/test/functional.js b/test/functional.js index 7e69fd60..c3522f56 100644 --- a/test/functional.js +++ b/test/functional.js @@ -39,7 +39,7 @@ function convertToManifestPath(assetSrc, webpackConfig) { describe('Functional tests using webpack', function() { // being functional tests, these can take quite long - this.timeout(5000); + this.timeout(8000); after(() => { testSetup.emptyTmpDir(); @@ -47,17 +47,25 @@ describe('Functional tests using webpack', function() { describe('Basic scenarios', () => { - it('Builds a simple .js file + manifest.json', (done) => { + it('Builds a few simple entries file + manifest.json', (done) => { const config = createWebpackConfig('web/build', 'dev'); config.addEntry('main', './js/no_require'); + config.addStyleEntry('font', './css/roboto_font.css'); + config.addStyleEntry('bg', './css/another_bg_image.css'); config.setPublicPath('/build'); testSetup.runWebpack(config, (webpackAssert) => { // should have a main.js file // should have a manifest.json with public/main.js - expect(config.outputPath).to.be.a.directory() - .with.files(['main.js', 'manifest.json']); + expect(config.outputPath).to.be.a.directory().with.deep.files([ + 'main.js', + 'font.css', + 'bg.css', + 'fonts/Roboto.woff2', + 'images/symfony_logo.png', + 'manifest.json' + ]); // check that main.js has the correct contents webpackAssert.assertOutputFileContains( @@ -73,6 +81,18 @@ describe('Functional tests using webpack', function() { 'build/main.js', '/build/main.js' ); + webpackAssert.assertManifestPath( + 'build/font.css', + '/build/font.css' + ); + webpackAssert.assertManifestPath( + 'build/fonts/Roboto.woff2', + '/build/fonts/Roboto.woff2' + ); + webpackAssert.assertManifestPath( + 'build/images/symfony_logo.png', + '/build/images/symfony_logo.png' + ); done(); });