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.

Sign up for GitHub

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

Webpack 5 for client mono-repo #10186

Merged
merged 9 commits into from
May 10, 2022
Merged

Webpack 5 for client mono-repo #10186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
656ece1
update to webpack 5
May 6, 2022
5ae35f9
fix spaces example
May 6, 2022
9c2eb73
Fix spaces tests
May 6, 2022
bad792f
use process polyfill
May 6, 2022
21ed32a
fix contact collection
May 9, 2022
feaa9fc
Merge branch 'main' of https://github.com/microsoft/FluidFramework in…
May 9, 2022
84db76f
Use contenthash
May 9, 2022
d34aa8b
fix several tests by polyfilling process
May 10, 2022
273f3ef
fix remaining tests
May 10, 2022
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
6 changes: 3 additions & 3 deletions azure/packages/external-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,17 @@
"eslint-plugin-react": "~7.28.0",
"eslint-plugin-tsdoc": "~0.2.14",
"eslint-plugin-unicorn": "~40.0.0",
"html-webpack-plugin": "^4.5.2",
"html-webpack-plugin": "^5.5.0",
"jest": "^26.6.3",
"jest-junit": "^10.0.0",
"jest-puppeteer": "^4.3.0",
"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"ts-jest": "^26.4.4",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
7 changes: 4 additions & 3 deletions examples/apps/collaborative-textarea/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,18 @@
"eslint-plugin-react": "~7.28.0",
"eslint-plugin-tsdoc": "~0.2.14",
"eslint-plugin-unicorn": "~40.0.0",
"html-webpack-plugin": "^4.5.2",
"html-webpack-plugin": "^5.5.0",
"jest": "^26.6.3",
"jest-junit": "^10.0.0",
"jest-puppeteer": "^4.3.0",
"process": "^0.11.10",
Copy link
Member

Choose a reason for hiding this comment

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

I'd like to track down why this polyfill is needed, so I can update #9508 if needed. Do you know where it's coming from?

I think our goal should be that our packages are usable in Webpack without adding polyfills to your config. As of now the plan is to disallow node polyfills except for events and url, but that goal is really focused on our libs. Examples could continue to use polyfills directly in Webpack like you do here, but that implies that our customers will too.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see now in a comment later:

We do however transitively depend on the util npm package (node_modules/util/util.js) which requires process.env to be defined. browserify/node-util#57 (comment)

I'll file an issue if you haven't already. At the very least this will need to be documented when using our packages with Webpack 5.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have not filed any issues about this. To me this looks like a bug in util (has an undeclared dependency), and a quality issue with whatever we use that depends on util (which would be nice for it not to depend on nodejs stuff). I'll leave how to track/address this to you.

When using webpack, I generally expect random transitive dependencies to need manual fiddling (it makes me sad this is something I have accepted), but I suppose trying to get this fixed could help our consumers and may be worth it.

Copy link
Member

Choose a reason for hiding this comment

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

I added #10225.

Copy link
Member

Choose a reason for hiding this comment

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

I'm somewhat surprised you didn't have to polyfill additional node libraries, especially events (#9509) and url (#9510). Do you know why that is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any polyfill that is transitively depended on some other way (like util, which is why we need process) is actually already being included and I don't know if that includes events and url, but it is possible.

It's also quite possible some behaviors are broken, and they were just missed by our automated tests and my manual testing: the issue with process only showed up at runtime (though our automated tests did catch it as well), not build or pack time, so we could be missing things that arn't tested and still pass CI.

"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"ts-jest": "^26.4.4",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
6 changes: 6 additions & 0 deletions examples/apps/collaborative-textarea/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const fluidRoute = require("@fluid-tools/webpack-fluid-loader");
const path = require("path");
const { merge } = require("webpack-merge");
const webpack = require("webpack");

module.exports = env => {
const isProduction = env && env.production;
Expand Down Expand Up @@ -40,6 +41,11 @@ module.exports = env => {
'Access-Control-Allow-Origin': '*'
},
},
plugins: [
new webpack.ProvidePlugin({
process: 'process/browser'
}),
],
// This impacts which files are watched by the dev server (and likely by webpack if watch is true).
// This should be configurable under devServer.static.watch
// (see https://github.com/webpack/webpack-dev-server/blob/master/migration-v4.md) but that does not seem to work.
Expand Down
4 changes: 4 additions & 0 deletions examples/apps/collaborative-textarea/webpack.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const path = require("path");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const webpack = require("webpack");

const pkg = require("./package.json");
const componentName = pkg.name.slice(1);
Expand Down Expand Up @@ -42,6 +43,9 @@ module.exports = env => {
}
},
plugins: [
new webpack.ProvidePlugin({
process: 'process/browser'
}),
new HtmlWebpackPlugin({
template: "./tests/index.html",
}),
Expand Down
7 changes: 4 additions & 3 deletions examples/apps/contact-collection/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,17 +75,18 @@
"eslint-plugin-react": "~7.28.0",
"eslint-plugin-tsdoc": "~0.2.14",
"eslint-plugin-unicorn": "~40.0.0",
"html-webpack-plugin": "^4.5.2",
"html-webpack-plugin": "^5.5.0",
"jest": "^26.6.3",
"jest-junit": "^10.0.0",
"jest-puppeteer": "^4.3.0",
"process": "^0.11.10",
"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"ts-jest": "^26.4.4",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
4 changes: 4 additions & 0 deletions examples/apps/contact-collection/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const path = require("path");
const { merge } = require("webpack-merge");
const webpack = require("webpack");
const HtmlWebpackPlugin = require("html-webpack-plugin");
// const { CleanWebpackPlugin } = require("clean-webpack-plugin");

Expand Down Expand Up @@ -34,6 +35,9 @@ module.exports = env => {
libraryTarget: "umd"
},
plugins: [
new webpack.ProvidePlugin({
process: 'process/browser'
}),
new HtmlWebpackPlugin({
template: "./src/index.html",
}),
Expand Down
4 changes: 4 additions & 0 deletions examples/apps/contact-collection/webpack.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const path = require("path");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const webpack = require("webpack");

module.exports = env => {
return ({
Expand Down Expand Up @@ -39,6 +40,9 @@ module.exports = env => {
}
},
plugins: [
new webpack.ProvidePlugin({
process: 'process/browser'
}),
new HtmlWebpackPlugin({
template: "./tests/index.html",
}),
Expand Down
9 changes: 5 additions & 4 deletions examples/apps/spaces/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,20 +92,21 @@
"eslint-plugin-tsdoc": "~0.2.14",
"eslint-plugin-unicorn": "~40.0.0",
"html-loader": "^3.1.0",
"html-webpack-plugin": "^4.5.2",
"html-webpack-plugin": "^5.5.0",
"jest": "^26.6.3",
"jest-junit": "^10.0.0",
"jest-puppeteer": "^4.3.0",
"process": "^0.11.10",
"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"sass-loader": "^7.1.0",
"source-map-loader": "^1.1.3",
"source-map-loader": "^2.0.0",
"style-loader": "^1.0.0",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"url-loader": "^2.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
23 changes: 16 additions & 7 deletions examples/apps/spaces/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const path = require("path");
const { merge } = require("webpack-merge");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const webpack = require("webpack");

const pkg = require("./package.json");
const componentName = pkg.name.slice(1);
Expand All @@ -19,6 +20,13 @@ module.exports = (env) => {
},
resolve: {
extensions: [".ts", ".tsx", ".js"],
fallback: {
dgram: false,
fs: false,
net: false,
tls: false,
child_process: false,
}
},
module: {
rules: [
Expand All @@ -35,13 +43,6 @@ module.exports = (env) => {
},
],
},
node: {
dgram: "empty",
fs: "empty",
net: "empty",
tls: "empty",
child_process: "empty",
},
output: {
filename: "[name].bundle.js",
path: path.resolve(__dirname, "dist"),
Expand All @@ -52,6 +53,14 @@ module.exports = (env) => {
libraryTarget: "umd",
},
plugins: [
// As of webpack 5, we no longer automatically get node polyfills.
// We do however transitively depend on the `util` npm package (node_modules/util/util.js) which requires `process.env` to be defined.
// We can explicitly load the polyfill for process to make this work:
// https://github.com/browserify/node-util/issues/57#issuecomment-764436352
// Note that using DefinePlugin with `process.env.NODE_DEBUG': undefined` would also handle this case.
new webpack.ProvidePlugin({
process: 'process/browser'
}),
new HtmlWebpackPlugin({
template: "./public/index.html",
}),
Expand Down
36 changes: 7 additions & 29 deletions examples/apps/spaces/webpack.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,47 +6,25 @@
const path = require("path");
const HtmlWebpackPlugin = require("html-webpack-plugin");

const pkg = require("./package.json");
const componentName = pkg.name.slice(1);

module.exports = env => {
return ({
const config = require("./webpack.config")(env);
return {
...config,
entry: {
app: "./tests/index.ts"
},
resolve: {
extensions: [".ts", ".tsx", ".js"],
},
module: {
rules: [{
test: /\.tsx?$/,
loader: require.resolve("ts-loader")
},
{
test: /\.css$/i,
use: [require.resolve('style-loader'), require.resolve('css-loader')],
}]
},
output: {
filename: "[name].bundle.js",
path: path.resolve(__dirname, "dist"),
library: "[name]",
// https://github.com/webpack/webpack/issues/5767
// https://github.com/webpack/webpack/issues/7939
devtoolNamespace: componentName,
libraryTarget: "umd"
},
mode: "development",
devtool: "inline-source-map",
devServer: {
static: {
directory: path.join(__dirname, 'tests')
}
},
plugins: [
config.plugins[0],
new HtmlWebpackPlugin({
template: "./tests/index.html",
}),
],
mode: "development",
devtool: "inline-source-map"
});
}
};
7 changes: 4 additions & 3 deletions examples/apps/view-framework-sampler/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,18 @@
"eslint-plugin-react": "~7.28.0",
"eslint-plugin-tsdoc": "~0.2.14",
"eslint-plugin-unicorn": "~40.0.0",
"html-webpack-plugin": "^4.5.2",
"html-webpack-plugin": "^5.5.0",
"jest": "^26.6.3",
"jest-junit": "^10.0.0",
"jest-puppeteer": "^4.3.0",
"process": "^0.11.10",
"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"ts-jest": "^26.4.4",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
4 changes: 4 additions & 0 deletions examples/apps/view-framework-sampler/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
const path = require("path");
const { merge } = require("webpack-merge");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const webpack = require("webpack");
// const { CleanWebpackPlugin } = require("clean-webpack-plugin");

module.exports = env => {
Expand Down Expand Up @@ -34,6 +35,9 @@ module.exports = env => {
libraryTarget: "umd"
},
plugins: [
new webpack.ProvidePlugin({
process: 'process/browser'
}),
new HtmlWebpackPlugin({
template: "./src/index.html",
}),
Expand Down
4 changes: 4 additions & 0 deletions examples/apps/view-framework-sampler/webpack.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

const path = require("path");
const HtmlWebpackPlugin = require("html-webpack-plugin");
const webpack = require("webpack");

module.exports = env => {
return ({
Expand Down Expand Up @@ -39,6 +40,9 @@ module.exports = env => {
}
},
plugins: [
new webpack.ProvidePlugin({
process: 'process/browser'
}),
new HtmlWebpackPlugin({
template: "./tests/index.html",
}),
Expand Down
4 changes: 2 additions & 2 deletions examples/data-objects/canvas/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,11 @@
"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"style-loader": "^1.0.0",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"url-loader": "^2.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
14 changes: 7 additions & 7 deletions examples/data-objects/canvas/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ module.exports = env => {
entry: {
main: "./src/index.ts"
},
node: {
dgram: 'empty',
fs: 'empty',
net: 'empty',
tls: 'empty',
child_process: 'empty',
},
resolve: {
extensions: [".ts", ".tsx", ".js"],
fallback: {
dgram: false,
fs: false,
net: false,
tls: false,
child_process: false,
}
},
module: {
rules: [{
Expand Down
4 changes: 2 additions & 2 deletions examples/data-objects/clicker/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@
"puppeteer": "^1.20.0",
"rimraf": "^2.6.2",
"ts-jest": "^26.4.4",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
4 changes: 2 additions & 2 deletions examples/data-objects/codemirror/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,10 @@
"eslint-plugin-unicorn": "~40.0.0",
"rimraf": "^2.6.2",
"style-loader": "^1.0.0",
"ts-loader": "^8.4.0",
"ts-loader": "^9.3.0",
"typescript": "~4.5.5",
"typescript-formatter": "7.1.0",
"webpack": "^4.46.0",
"webpack": "^5.72.0",
"webpack-cli": "^4.9.2",
"webpack-dev-server": "4.0.0",
"webpack-merge": "^5.8.0"
Expand Down
Loading