Skip to content

Commit 9d859da

Browse files
weaverryanweaverryan
committed
bug #453 [BC Break] Fixing a bug where the entrypoint path opening slash was stripped (weaverryan)
This PR was merged into the master branch. Discussion ---------- [BC Break] Fixing a bug where the entrypoint path opening slash was stripped Fixes #452 When `entrypoints.json` was originally designed, the values were made so that they matched the *keys* of `manifest.json` - the idea was that you would use both to get the final path. But later (before 0.21.0 releaser), we/I changed `entrypoints.json` so that the value is the final path - to avoid needing to do another lookup in `manifest.json`. However, there was one piece that I missed when changing this: the opening `/` is currently being stripped off the values in `entrypoints.json`. This breaks subdirectory installs (see #452). For most Symfony users, you won't notice this change. **Current behavior**: The value is read from `entrypoints.json` (`build/foo.js`), then it is also found in `manifest.json` (via the `json_manifest_strategy`) and becomes `/build/foo.js`. **New behavior**: The value is read from `entrypoints.json` (`/build/foo.js`) and that's it. In both cases, the final value that enters into "Symfony" is `/build/foo.js`. But, for anyone not using the standard Symfony setup, this is a behavior change, as the values from `entrypoints.json` ARE changing. Commits ------- 6190551 Fixing a bug where the entrypoint path opening slash was stripped
2 parents 2a73fbc + 6190551 commit 9d859da

File tree

2 files changed

+66
-26
lines changed

2 files changed

+66
-26
lines changed

‎lib/plugins/entry-files-manifest.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,13 @@ function processOutput(assets) {
2626
// like normal below
2727
delete assets.entrypoints;
2828

29-
// This will iterate over all the entry points and remove the / from the start of the paths. It also converts the
29+
// This will iterate over all the entry points and convert the
3030
// one file entries into an array of one entry since that was how the entry point file was before this change.
3131
for (const asset in assets) {
3232
for (const fileType in assets[asset]) {
3333
if (!Array.isArray(assets[asset][fileType])) {
3434
assets[asset][fileType] = [assets[asset][fileType]];
3535
}
36-
37-
assets[asset][fileType] = assets[asset][fileType].map(buildPath => buildPath.replace(/^\//g, ''));
3836
}
3937
}
4038

‎test/functional.js

+65-23
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,15 @@ describe('Functional tests using webpack', function() {
104104
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
105105
entrypoints: {
106106
main: {
107-
js: ['build/runtime.js', 'build/main.js']
107+
js: ['/build/runtime.js', '/build/main.js']
108108
},
109109
font: {
110-
js: ['build/runtime.js'],
111-
css: ['build/font.css']
110+
js: ['/build/runtime.js'],
111+
css: ['/build/font.css']
112112
},
113113
bg: {
114-
js: ['build/runtime.js'],
115-
css: ['build/bg.css']
114+
js: ['/build/runtime.js'],
115+
css: ['/build/bg.css']
116116
}
117117
}
118118
});
@@ -135,12 +135,12 @@ describe('Functional tests using webpack', function() {
135135
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
136136
entrypoints: {
137137
main: {
138-
js: ['build/runtime.js', 'build/vendors~main~other.js', 'build/main~other.js', 'build/main.js'],
139-
css: ['build/main~other.css']
138+
js: ['/build/runtime.js', '/build/vendors~main~other.js', '/build/main~other.js', '/build/main.js'],
139+
css: ['/build/main~other.css']
140140
},
141141
other: {
142-
js: ['build/runtime.js', 'build/vendors~main~other.js', 'build/main~other.js', 'build/other.js'],
143-
css: ['build/main~other.css']
142+
js: ['/build/runtime.js', '/build/vendors~main~other.js', '/build/main~other.js', '/build/other.js'],
143+
css: ['/build/main~other.css']
144144
}
145145
}
146146
});
@@ -695,12 +695,12 @@ describe('Functional tests using webpack', function() {
695695
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
696696
entrypoints: {
697697
main: {
698-
js: ['build/runtime.js', 'build/shared.js', 'build/main.js'],
699-
css: ['build/shared.css']
698+
js: ['/build/runtime.js', '/build/shared.js', '/build/main.js'],
699+
css: ['/build/shared.css']
700700
},
701701
other: {
702-
js: ['build/runtime.js', 'build/shared.js', 'build/other.js'],
703-
css: ['build/shared.css']
702+
js: ['/build/runtime.js', '/build/shared.js', '/build/other.js'],
703+
css: ['/build/shared.css']
704704
}
705705
}
706706
});
@@ -1621,12 +1621,12 @@ module.exports = {
16211621
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
16221622
entrypoints: {
16231623
main: {
1624-
js: ['build/runtime.js', 'build/vendors~main~other.js', 'build/main~other.js', 'build/main.js'],
1625-
css: ['build/main~other.css']
1624+
js: ['/build/runtime.js', '/build/vendors~main~other.js', '/build/main~other.js', '/build/main.js'],
1625+
css: ['/build/main~other.css']
16261626
},
16271627
other: {
1628-
js: ['build/runtime.js', 'build/vendors~main~other.js', 'build/main~other.js', 'build/other.js'],
1629-
css: ['build/main~other.css']
1628+
js: ['/build/runtime.js', '/build/vendors~main~other.js', '/build/main~other.js', '/build/other.js'],
1629+
css: ['/build/main~other.css']
16301630
}
16311631
}
16321632
});
@@ -1680,6 +1680,48 @@ module.exports = {
16801680
});
16811681
});
16821682

1683+
it('Subdirectory public path affects entrypoints.json but does not affect manifest.json', (done) => {
1684+
const config = createWebpackConfig('web/build', 'dev');
1685+
config.addEntry('main', ['./css/roboto_font.css', './js/no_require', 'vue']);
1686+
config.addEntry('other', ['./css/roboto_font.css', 'vue']);
1687+
config.setPublicPath('/subdirectory/build');
1688+
config.setManifestKeyPrefix('custom_prefix');
1689+
config.configureSplitChunks((splitChunks) => {
1690+
splitChunks.chunks = 'all';
1691+
splitChunks.minSize = 0;
1692+
});
1693+
1694+
testSetup.runWebpack(config, (webpackAssert) => {
1695+
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
1696+
entrypoints: {
1697+
main: {
1698+
js: [
1699+
'/subdirectory/build/runtime.js',
1700+
'/subdirectory/build/vendors~main~other.js',
1701+
'/subdirectory/build/main~other.js',
1702+
'/subdirectory/build/main.js'
1703+
],
1704+
css: ['/subdirectory/build/main~other.css']
1705+
},
1706+
other: {
1707+
js: [
1708+
'/subdirectory/build/runtime.js',
1709+
'/subdirectory/build/vendors~main~other.js',
1710+
'/subdirectory/build/main~other.js',
1711+
'/subdirectory/build/other.js'
1712+
],
1713+
css: ['/subdirectory/build/main~other.css']
1714+
}
1715+
}
1716+
});
1717+
1718+
// make split chunks are correct in manifest
1719+
webpackAssert.assertManifestKeyExists('custom_prefix/vendors~main~other.js');
1720+
1721+
done();
1722+
});
1723+
});
1724+
16831725
it('Use splitChunks in production mode', (done) => {
16841726
const config = createWebpackConfig('web/build', 'production');
16851727
config.addEntry('main', ['./css/roboto_font.css', './js/no_require', 'vue']);
@@ -1695,12 +1737,12 @@ module.exports = {
16951737
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
16961738
entrypoints: {
16971739
main: {
1698-
js: ['build/runtime.js', 'build/vendors~cc515e6e.js', 'build/default~cc515e6e.js', 'build/main.js'],
1699-
css: ['build/default~cc515e6e.css']
1740+
js: ['/build/runtime.js', '/build/vendors~cc515e6e.js', '/build/default~cc515e6e.js', '/build/main.js'],
1741+
css: ['/build/default~cc515e6e.css']
17001742
},
17011743
other: {
1702-
js: ['build/runtime.js', 'build/vendors~cc515e6e.js', 'build/default~cc515e6e.js', 'build/other.js'],
1703-
css: ['build/default~cc515e6e.css']
1744+
js: ['/build/runtime.js', '/build/vendors~cc515e6e.js', '/build/default~cc515e6e.js', '/build/other.js'],
1745+
css: ['/build/default~cc515e6e.css']
17041746
}
17051747
}
17061748
});
@@ -1726,12 +1768,12 @@ module.exports = {
17261768
webpackAssert.assertOutputJsonFileMatches('entrypoints.json', {
17271769
entrypoints: {
17281770
main: {
1729-
js: ['build/runtime.js', 'build/vendors~main~other.js', 'build/main.js']
1771+
js: ['/build/runtime.js', '/build/vendors~main~other.js', '/build/main.js']
17301772
},
17311773
other: {
17321774
// the 0.[hash].js is because the "no_require" module was already split to this
17331775
// so, it has that filename, instead of following the normal pattern
1734-
js: ['build/runtime.js', 'build/vendors~main~other.js', 'build/0.js', 'build/other.js']
1776+
js: ['/build/runtime.js', '/build/vendors~main~other.js', '/build/0.js', '/build/other.js']
17351777
}
17361778
}
17371779
});

0 commit comments

Comments
 (0)