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

[hold] upgrade: Webpack 5 #6496

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 36 additions & 40 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@
"@artsy/palette": "13.15.0",
"@artsy/passport": "1.5.0",
"@artsy/reaction": "28.11.0",
"@artsy/stitch": "6.1.6",
"@artsy/stitch": "^6.2.0",
"@artsy/xapp": "1.0.6",
"@loadable/component": "5.12.0",
"@loadable/server": "5.13.2",
"@loadable/component": "5.14.1",
"@loadable/server": "5.14.0",
"@sentry/browser": "4.5.4",
"@styled-system/theme-get": "5.1.2",
"accounting": "0.4.1",
Expand Down Expand Up @@ -219,23 +219,22 @@
},
"devDependencies": {
"@artsy/antigravity": "0.2.0",
"@babel/cli": "7.10.5",
"@babel/core": "7.11.1",
"@babel/plugin-proposal-class-properties": "7.10.4",
"@babel/plugin-proposal-decorators": "7.10.5",
"@babel/plugin-proposal-nullish-coalescing-operator": "7.10.4",
"@babel/plugin-proposal-optional-chaining": "7.11.0",
"@babel/plugin-syntax-dynamic-import": "7.8.3",
"@babel/plugin-transform-modules-commonjs": "7.10.4",
"@babel/plugin-transform-runtime": "7.11.0",
"@babel/preset-env": "7.11.0",
"@babel/preset-react": "7.10.4",
"@babel/preset-typescript": "7.10.4",
"@babel/register": "7.10.5",
"@babel/runtime": "7.11.2",
"@babel/cli": "^7.12.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a good idea to extract these babel updates into their own PR if we can easily isolate.

"@babel/core": "^7.12.3",
"@babel/plugin-proposal-class-properties": "^7.12.1",
"@babel/plugin-proposal-decorators": "^7.12.1",
"@babel/plugin-proposal-nullish-coalescing-operator": "^7.12.1",
"@babel/plugin-proposal-optional-chaining": "^7.12.1",
"@babel/plugin-transform-modules-commonjs": "^7.12.1",
"@babel/plugin-transform-runtime": "^7.12.1",
"@babel/preset-env": "^7.12.1",
"@babel/preset-react": "^7.12.1",
"@babel/preset-typescript": "^7.12.1",
"@babel/register": "^7.12.1",
"@babel/runtime": "^7.12.1",
"@graphql-inspector/core": "1.14.0",
"@loadable/babel-plugin": "5.12.0",
"@loadable/webpack-plugin": "5.12.0",
"@loadable/babel-plugin": "^5.13.2",
"@loadable/webpack-plugin": "^5.14.0",
"@sentry/cli": "1.41.2",
"@types/dd-trace": "0.6.0",
"@types/dedent": "0.7.0",
Expand Down Expand Up @@ -267,20 +266,19 @@
"@types/styled-system": "5.1.9",
"@types/styled-system__theme-get": "5.0.0",
"@types/underscore.string": "0.0.32",
"@types/webpack": "4.4.24",
"@types/webpack-env": "1.13.6",
"@types/webpack-env": "1.15.3",
"@typescript-eslint/eslint-plugin": "2.30.0",
"@typescript-eslint/parser": "2.30.0",
"babel-jest": "24.8.0",
"babel-loader": "8.0.6",
"babel-plugin-inline-react-svg": "0.2.0",
"babel-plugin-lodash": "3.3.4",
"babel-plugin-module-resolver": "3.1.0",
"babel-plugin-relay": "9.0.0",
"babel-plugin-styled-components": "1.11.1",
"babel-jest": "^26.6.2",
"babel-loader": "^8.1.0",
"babel-plugin-inline-react-svg": "^1.1.2",
"babel-plugin-lodash": "^3.3.4",
"babel-plugin-module-resolver": "^4.0.0",
"babel-plugin-relay": "^10.0.1",
"babel-plugin-styled-components": "^1.11.1",
"benv": "3.3.0",
"buffer": "5.6.0",
"cache-loader": "1.2.2",
"case-sensitive-paths-webpack-plugin": "2.3.0",
"coffee-loader": "0.8.0",
"coffeescript": "1.11.1",
"cypress": "5.1.0",
Expand All @@ -294,9 +292,7 @@
"eslint-plugin-react": "7.19.0",
"eslint-plugin-react-hooks": "4.0.4",
"eslint-plugin-styled-components-a11y": "0.0.16",
"fork-ts-checker-notifier-webpack-plugin": "0.4.0",
"fork-ts-checker-webpack-plugin": "0.4.10",
"friendly-errors-webpack-plugin": "1.6.1",
"graphql-tag": "2.10.3",
"graphql-tools": "4.0.3",
"hulk-editor": "craigspaeth/hulk",
Expand All @@ -320,6 +316,7 @@
"mocha-multi-reporters": "1.1.7",
"nightmare": "2.10.0",
"nyc": "13.3.0",
"os-browserify": "0.3.0",
"patch-package": "6.2.0",
"postinstall-prepare": "1.0.1",
"prettier": "2.0.5",
Expand All @@ -335,27 +332,26 @@
"relay-test-utils": "9.1.0",
"rewire": "2.2.0",
"should": "11.2.1",
"simple-progress-webpack-plugin": "1.1.2",
"sinon": "1.17.7",
"speed-measure-webpack-plugin": "1.3.3",
"static-extend": "0.1.2",
"strip-ansi": "5.0.0",
"style-loader": "0.23.1",
"stylus-loader": "3.0.2",
"supertest": "2.0.1",
"terser-webpack-plugin": "4.1.0",
"terser-webpack-plugin": "5.0.0",
"typescript": "3.9.7",
"typescript-styled-plugin": "0.15.0",
"vscode-apollo-relay": "1.5.1",
"webpack": "4.44.1",
"webpack-bundle-analyzer": "3.6.0",
"webpack-cli": "3.3.12",
"webpack": "5.3.2",
"webpack-bundle-analyzer": "3.9.0",
"webpack-cli": "4.0.0",
"webpack-config-utils": "2.3.1",
"webpack-dev-middleware": "3.5.1",
"webpack-hot-middleware": "2.24.3",
"webpack-manifest-plugin": "2.0.4",
"webpack-dev-middleware": "3.7.2",
"webpack-hot-middleware": "2.25.0",
"webpack-manifest-plugin": "3.0.0-rc.0",
"webpack-merge": "4.2.1",
"webpack-node-externals": "1.7.2",
"webpack-node-externals": "2.5.2",
"webpack-notifier": "1.7.0",
"webpack-retry-chunk-load-plugin": "1.2.0",
"yn": "4.0.0"
Expand Down
5 changes: 3 additions & 2 deletions src/desktop/apps/auction/components/layout/Footer.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import PropTypes from "prop-types"
import React from "react"
import block from "bem-cn-lite"
import renderTemplate from "../../utils/renderTemplate"
import { connect } from "react-redux"
import { first } from "underscore"
import { first } from "lodash"

function Footer(props) {
const { articles, footerItem, showArticles, showFooterItems, sd } = props
Expand All @@ -28,6 +27,8 @@ function Footer(props) {

// Serverside
if (typeof window === "undefined") {

const renderTemplate = require("../../utils/renderTemplate")
const path = require("path")

articleFigureHTML = renderTemplate(
Expand Down
21 changes: 0 additions & 21 deletions src/desktop/apps/auction/utils/renderTemplate.js

This file was deleted.

29 changes: 29 additions & 0 deletions src/desktop/apps/auction/utils/renderTemplate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import jade from "jade"
import path from "path"
import { first, isArray } from "lodash"

type RenderOptions = {
basePath?: string,
compilerOptions?: object,
locals?: object,
}

export default function renderTemplate(templates: string, options: RenderOptions): string;
export default function renderTemplate(templates: string[], options: RenderOptions): string[];
export default function renderTemplate(templates: string | string[], options: RenderOptions = {}): string | string[] {
const normalizedTemplates = isArray(templates) ? templates : [templates]

const { basePath = "", compilerOptions = {}, locals = {} } = options

const templateFns = normalizedTemplates.map(file => {
return jade.compileFile(path.join(basePath, file), compilerOptions) as (obj: any) => string
})

const renderedTemplates = templateFns.map(template => template(locals))

// If user only passed in a single template, return a single template
const out =
renderedTemplates.length > 1 ? renderedTemplates : first(renderedTemplates)

return out
}
1 change: 0 additions & 1 deletion src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ if (NODE_ENV === "development") {
require("coffeescript/register")
require("@babel/register")({
extensions: [".ts", ".js", ".tsx", ".jsx"],
plugins: ["babel-plugin-dynamic-import-node"],
})
}

Expand Down
4 changes: 2 additions & 2 deletions src/v2/Artsy/Analytics/useRouteTracking.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ export function useRouteTracking() {

// Wait till after the next tick to set to false, to give react tree
// ability to execute related tracking handlers
setImmediate(() => {
setTimeout(() => {
setShouldTrack(false)
})
}, 0)
}
}, [isFetching, prevFetching])

Expand Down
34 changes: 17 additions & 17 deletions webpack/envs/clientCommonConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ export const clientCommonConfig = {
},
],
},
// ESM support. See: https://github.com/apollographql/react-apollo/issues/1737#issuecomment-371178602
// Required for webpack 5 to allow interop with with non-ESM modules is handled.
// TODO: This may be removed once all dependant references to @babel/runtime-corejs3 are removed.
{
type: "javascript/auto",
test: /\.mjs$/,
use: [],
test: /\.m?js/,
resolve: {
fullySpecified: false,
},
},
],
},
Expand All @@ -75,20 +77,13 @@ export const clientCommonConfig = {
},
}),
// Remove moment.js localization files
new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/),
new webpack.IgnorePlugin({
resourceRegExp: /^\.\/locale$/,
contextRegExp: /moment$/,
}),
Copy link
Member

Choose a reason for hiding this comment

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

I think this was added due to how Artsy/Router/buildServer|ClientApp used to be exported from reaction, but we import these things directly everywhere now so i think this is safe to remove.

// Remove server-only modules from client bundles
...[
// Remove server side of relay network layer.
new webpack.IgnorePlugin(
/^react-relay-network-modern-ssr\/node8\/server/
),
// No matter what, we don't want the graphql-js package in client
// bundles. This /may/ lead to a broken build when e.g. a reaction
// module that's used on the client side imports something from
// graphql-js, but that's better than silently including this.
new webpack.IgnorePlugin(/^graphql(\/.*)?$/),
],
new webpack.NamedModulesPlugin(),
// TODO: Why would these end up in the client bundle?
new webpack.IgnorePlugin({ resourceRegExp: /^graphql(\/.*)?$/ }),
new webpack.ProvidePlugin({
$: "jquery",
jQuery: "jquery",
Expand Down Expand Up @@ -135,6 +130,11 @@ export const clientCommonConfig = {
// Symlink issues should be fixed via `yarn --pnp`
modules: [path.resolve(basePath, "src"), "node_modules"],
symlinks: false,
fallback: {
path: false,
os: require.resolve("os-browserify/browser"),
buffer: require.resolve("buffer/"),
},
},
optimization: {
// Extract webpack runtime code into it's own file
Expand Down
22 changes: 1 addition & 21 deletions webpack/envs/clientDevelopmentConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,8 @@ const chalk = require("chalk")
const fs = require("fs")
const path = require("path")
const webpack = require("webpack")
const ForkTsCheckerNotifierWebpackPlugin = require("fork-ts-checker-notifier-webpack-plugin")
const ForkTsCheckerWebpackPlugin = require("fork-ts-checker-webpack-plugin")
const FriendlyErrorsWebpackPlugin = require("friendly-errors-webpack-plugin")
const WebpackNotifierPlugin = require("webpack-notifier")
const SimpleProgressWebpackPlugin = require("simple-progress-webpack-plugin")
const CaseSensitivePathsPlugin = require("case-sensitive-paths-webpack-plugin")
const { basePath, env } = require("../utils/env")

const cacheDirectory = path.resolve(basePath, ".cache")
Expand All @@ -18,7 +14,7 @@ if (!env.onCi && !fs.existsSync(cacheDirectory)) {
console.log(
chalk.yellow(
"\n[!] No existing `.cache` directory detected, initial " +
"launch will take a while.\n"
"launch will take a while.\n"
)
)
}
Expand Down Expand Up @@ -56,29 +52,13 @@ export const clientDevelopmentConfig = {
],
},
plugins: [
new CaseSensitivePathsPlugin(),
Copy link
Member

Choose a reason for hiding this comment

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

This was recently added because it helped us track down a very pesky case sensitive bug. Why remove?

Copy link
Member

@damassi damassi Oct 30, 2020

Choose a reason for hiding this comment

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

If all of the following removals are due to plugin compat, we should comment them out with a

TODO: Reenable once webpack 5 is supported

unless there are comparable built-in webpack features we can leverage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you explain the particular bug in more detail or provide a link to it? I am intimately familiar with Apples case-insensitive file system and the many issues that it creates in particular when renaming files in git. However git has options to address this already and wonder if we could instead leverage those.

Copy link
Member

@dzucconi dzucconi Oct 30, 2020

Choose a reason for hiding this comment

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

#6462 was the issue that prompted this — I had downcased a folder name accidentally against convention, but imports would work locally with alternate casing. I'd very much like to be warned about this, regardless of where it happens.

Copy link
Member

Choose a reason for hiding this comment

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

(Er, that's not the issue but rather the fix)

new webpack.HotModuleReplacementPlugin(),
// @ts-ignore
new SimpleProgressWebpackPlugin({
format: "compact",
}),
new ForkTsCheckerWebpackPlugin({
formatter: "codeframe",
formatterOptions: "highlightCode",
checkSyntacticErrors: true,
watch: ["./src"],
}),
new ForkTsCheckerNotifierWebpackPlugin({
excludeWarnings: true,
skipFirstNotification: true,
}),
new FriendlyErrorsWebpackPlugin({
clearConsole: false,
compilationSuccessInfo: {
messages: [`[Force] Listening on http://localhost:${env.port} \n`],
notes: [""],
},
}),
new WebpackNotifierPlugin(),
],
}
6 changes: 2 additions & 4 deletions webpack/envs/clientProductionConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const path = require("path")
const WebpackManifestPlugin = require("webpack-manifest-plugin")
const { HashedModuleIdsPlugin } = require("webpack")
const webpack = require("webpack")
const { getCSSManifest } = require("../utils/getCSSManifest")
const TerserPlugin = require("terser-webpack-plugin")
const { basePath, env } = require("../utils/env")
Expand All @@ -18,7 +18,7 @@ export const clientProductionConfig = {
// @see: https://github.com/artsy/force/blob/master/src/desktop/lib/global_client_setup.tsx#L7
},
plugins: [
new HashedModuleIdsPlugin(),
new webpack.ids.HashedModuleIdsPlugin(),
new WebpackManifestPlugin({
fileName: path.resolve(basePath, "manifest.json"),
basePath: "/assets/",
Expand All @@ -29,9 +29,7 @@ export const clientProductionConfig = {
minimize: !env.webpackDebug,
minimizer: [
new TerserPlugin({
cache: false,
parallel: env.onCi ? env.webpackCiCpuLimit : true, // Only use 4 cpus (default) in CircleCI, by default it will try using 36 and OOM
sourceMap: true, // Must be set to true if using source-maps in production
}),
],
},
Expand Down
6 changes: 2 additions & 4 deletions webpack/envs/serverConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,16 @@ export const serverConfig = {
minimize: env.isProduction && !env.webpackDebug,
minimizer: [
new TerserPlugin({
cache: false,
damassi marked this conversation as resolved.
Show resolved Hide resolved
parallel: env.onCi ? env.webpackCiCpuLimit : true, // Only use 4 cpus (default) in CircleCI, by default it will try using 36 and OOM
sourceMap: true, // Must be set to true if using source-maps in production
}),
],
},
plugins: removeEmpty([
env.enableWebpackAnalyze
? undefined
: new webpack.optimize.LimitChunkCountPlugin({
maxChunks: 1,
}),
maxChunks: 1,
}),
]),
resolve: {
extensions: [".js", ".jsx", ".ts", ".tsx", ".json", ".coffee"],
Expand Down
2 changes: 1 addition & 1 deletion webpack/plugins/bundleAnalyzer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function bundleAnalyzer(config) {
}

return merge.smart(config, {
devtool: "none",
devtool: false,
stats: "normal",
optimization: {
concatenateModules: false,
Expand Down
Loading