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.

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

Update major dependencies - part 2/? #82

Merged
merged 10 commits into from
Nov 10, 2020

Conversation

brophdawg11
Copy link
Owner

No description provided.

@brophdawg11 brophdawg11 changed the title Update major dependencies (part 2) Update major dependencies - part 2/? Nov 9, 2020
@@ -29,6 +29,9 @@ function getCssLoaders(config) {
{
loader: 'css-loader',
options: {
// Required to work with css-loader@4
// https://github.com/vuejs/vue-style-loader/issues/46#issuecomment-670624576
esModule: false,
Copy link
Owner Author

@brophdawg11 brophdawg11 Nov 9, 2020

Choose a reason for hiding this comment

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

Looks like this was a breaking change in v4 that didn't play nicely with vue-style-loader. See:

@@ -47,7 +47,7 @@
"postcss-loader": "4.0.4",
"sass-loader": "8.0.2",
"svg-inline-loader": "0.8.2",
"svg-url-loader": "3.0.3",
"svg-url-loader": "6.0.0",
Copy link
Owner Author

Choose a reason for hiding this comment

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

versions 4 and 5 were just because the underlying dependency on file-loader underwent a major bump - but not API changes. Version 6 sets a default stripDeclarations: true option which should be fine for us.

https://github.com/bhovhannes/svg-url-loader/releases/tag/v6.0.0

@@ -44,7 +44,7 @@
"mini-css-extract-plugin": "1.3.0",
"node-sass": "4.13.1",
"optimize-css-assets-webpack-plugin": "5.0.3",
"postcss-loader": "3.0.0",
"postcss-loader": "4.0.4",
Copy link
Owner Author

Choose a reason for hiding this comment

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

Seemingly nothing that impacts us - we were already on required webpack/node versions. Out usage of a postcss config file also means we don't need to worry about the postcssOptions loader change:

https://github.com/webpack-contrib/postcss-loader/blob/master/CHANGELOG.md#400-2020-09-07

@@ -49,7 +49,7 @@
"svg-inline-loader": "0.8.2",
"svg-url-loader": "6.0.0",
"tar": "4.4.8",
"terser-webpack-plugin": "2.3.8",
"terser-webpack-plugin": "4.2.3",
Copy link
Owner Author

Choose a reason for hiding this comment

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

Only go to v4 - v5 drops support for webpack@4

Copy link
Owner Author

Choose a reason for hiding this comment

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

if (!cssLoaders[0].options.modules) {
cssLoaders[0].options.modules = {};
}
cssLoaders[0].options.modules.exportOnlyLocals = true;
Copy link
Owner Author

Choose a reason for hiding this comment

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

let ready;

/* eslint-disable global-require, import/no-dynamic-require */
const clientConfig = require(config.clientConfig);
const serverConfig = require(config.serverConfig);
/* eslint-enable global-require, import/no-dynamic-require */

const readFile = (_fs, file) => {
try {
return _fs.readFileSync(file, 'utf-8');
Copy link
Owner Author

Choose a reason for hiding this comment

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

Got rid of this utility to just inline the readFileSync calls below

@@ -11,27 +11,17 @@ const webpackDevMiddleware = require('webpack-dev-middleware');
/* eslint-enable import/no-extraneous-dependencies */

module.exports = function setupDevServer(app, config, cb) {

const mfs = new MFS();
Copy link
Owner Author

Choose a reason for hiding this comment

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

we will share an MFS between client and server builds now

@@ -79,10 +65,7 @@ module.exports = function setupDevServer(app, config, cb) {
if (json.errors.length) {
return;
}
clientManifest = JSON.parse(readFile(
devMiddleware.fileSystem,
Copy link
Owner Author

Choose a reason for hiding this comment

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

The API changed so you can no longer access this filesystem via the returned devMiddleware. So we now pass outputFileSystem: mfs and directly refer to mfs to read the updates files

Base automatically changed from brophdawg11/update-deps-major-1 to master November 10, 2020 15:55
@brophdawg11 brophdawg11 force-pushed the brophdawg11/update-deps-major-2 branch from 135cc56 to 8ab9c8d Compare November 10, 2020 19:09
@@ -41,15 +41,15 @@
"jest-serializer-vue": "2.0.2",
"lodash-es": "4.17.15",
"lru-cache": "6.0.0",
"mini-css-extract-plugin": "github:brophdawg11/mini-css-extract-plugin#urbn",
"mini-css-extract-plugin": "1.3.0",
Copy link
Owner Author

Choose a reason for hiding this comment

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

Switch back to the normal package now that our required functionality is available.

The fork we were pointing to added an insertInto option to control the ordering of injected extracted CSS stylesheets. This approach was subsequently replaced by a more flexible insert API so now we can switch back to the normal package and away from the fork.

See also https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/CHANGELOG.md#110-2020-10-19

Note we we apply the cssInsert vue-ssr-build option to the underlying mini-css-extract-plugin insert option: https://github.com/brophdawg11/vue-ssr-build/pull/82/files#diff-c2ab562cb8834a1a0f651353e7e8d264a26ea087d7f79e8e8a94f846212437f0R190

@brophdawg11 brophdawg11 merged commit d7d0341 into master Nov 10, 2020
@brophdawg11 brophdawg11 deleted the brophdawg11/update-deps-major-2 branch November 10, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants