[heft-web-rig] Add an "app" profile, upgrade to Webpack 5, implement new CSS rules #3204

octogonz · 2022-02-04T18:10:42Z

Summary

Update @rushstack/heft-web-rig with a bunch of new features.

Details

Upgrade to Webpack 5
Upgrade all the toolchain dependencies
Enable useUnknownInCatchVariables": false to work around Allow "catch (e as Error)" instead of "catch (e: unknown)" TypeScript#42596
Implement all the SASS/CSS improvements proposed in Improve heft-webpack-basic-tutorial to illustrate .scss support rushstack-samples#3
Integrate HBO's approach of inheriting webpack.config.js defaults from the rig using webpack-merge
Add rules for mini-css-extract-plugin, css-minimizer-webpack-plugin, webpack-bundle-analyzer
Use Import.resolveModule() to ensure require('webpack') finds the correct instance

Related ideas that are outside the scope of this PR:

Replace node-sass with sass for heft-sass-plugin
Replace lib-commonjs with emitCjsExtensionForCommonJS
Enable Webpack bundling of web libraries
Add more profiles to @rushstack/heft-node-rig

strictPeerDependencies fix

While trying to get the various Webpack 5 plugin versions straightened out, I encountered a bunch of peer dependency issues. The strictPeerDependencies validation for this repo apparently got disabled by accident a few months ago, and a bunch of warnings accumulated in the interim. Ideally that should be solved in a separate PR with its own code review, but in the interest of time it is more efficient to solve the equation with the final set of package.json files, so I've rolled it into this PR. Sorry for the huge diff. :-)

How it was tested

The new @rushstack/heft-web-rig was prototyped in the rushstack.io-service repo.

CC @iclanton @elliot-nelson @dmichon-msft

rigs/heft-web-rig/shared/webpack-base.config.js

octogonz · 2022-02-05T02:32:50Z

rigs/heft-web-rig/shared/webpack-base.config.js

+                  localIdentName: production ? '[hash:base64]' : '[local]__[hash:base64:5]'
+                },
+
+                sourceMap: !production


One weirdness of the above approach is that global .css files get sourcemaps injected, which you can see in the Chrome F12 debugger. (Does that even make sense when CSS modules are not being used?)

However because we are using 'mini-css-extract-plugin', the problem disappears when the CSS is extracted into a separate .css bundle.

octogonz · 2022-02-05T02:34:44Z

rigs/heft-web-rig/shared/webpack-base.config.js

+        DEBUG: !production
+      }),
+
+      process.env.REPORT ? new BundleAnalyzerPlugin({ analyzerMode: 'static' }) : undefined


The idea is to generate the .html reports optionally, only if the REPORT environment variable is set. (Other ideas welcome.) Currently we are using dotenv at HBO.

octogonz · 2022-02-05T02:35:00Z

rigs/heft-web-rig/shared/webpack-base.config.js

+    // this allows projects to override the default rig behavior if applicable (ex. when targeting older browsers)
+    customizeArray: unique(
+      'plugins',
+      ['HtmlWebpackPlugin'],


(This is slightly awkward -- is there a better way?)

I'm starting to think we should invent a more rigorous inheritance mechanism for webpack.config.js than these copy+paste createWebpackConfig() functions. When we tackle that, we can also figure out a better way to handle these overrides.

… peer dependency problems

…ft-web-rig-revamp

build-tests-samples/heft-storybook-react-tutorial/package.json

build-tests/heft-sass-test/package.json

common/changes/@rushstack/heft-web-rig/octogonz-heft-web-rig-revamp_2022-02-05-01-55.json

heft-plugins/heft-webpack5-plugin/package.json

iclanton · 2022-02-06T04:39:50Z

rigs/heft-web-rig/profiles/app/config/rush-project.json

+      "outputFolderNames": ["dist", "lib", "lib-commonjs", "temp"]
+    },
+    {
+      "operationName": "build",
+      "outputFolderNames": ["dist", "lib", "lib-commonjs", "temp"]


These lists should probably match the folders that heft clean deletes.

Good feedback, fixed!

Not in scope for this, but we should update Rush to always clean the output folders if it is not skipping the operation, not only if there is a cache hit. That way we get consistent behavior regarding rush cleaning regardless of the build cache.

iclanton · 2022-02-06T04:53:27Z

rigs/heft-web-rig/shared/webpack-base.config.js

+        {
+          // The source-map-loader extracts existing source maps from all JavaScript entries. This includes both
+          // inline source maps as well as those linked via URL. All source map data is passed to Webpack for
+          // processing as per a chosen source map style specified by the devtool option in webpack.config.js.
+          // https://www.npmjs.com/package/source-map-loader
+          test: /\.js$/,
+          include: [path.resolve(projectRoot, 'lib')],
+          enforce: 'pre',
+          use: [
+            {
+              loader: require.resolve('source-map-loader')
+            }
+          ]
+        },


This isn't built-in to Webpack at this point?

This rule still appears in the docs for Webpack 5:

https://webpack.js.org/loaders/source-map-loader/#getting-started

👉 Seems we should keep the above config as-is.

rigs/heft-web-rig/shared/webpack-base.config.js

iclanton · 2022-02-06T04:57:14Z

rigs/heft-web-rig/shared/webpack-base.config.js

+                {
+                  // Compiles SASS syntax into CSS
+                  // https://www.npmjs.com/package/sass-loader
+                  loader: require.resolve('sass-loader'),


I kinda think our recommended approach should be to compile SASS to CSS with heft-sass-plugin so we don't have to parse it twice.

@iclanton is that possible today?

Ian said that Heft has this feature today. However, compiling src/*.scss -> lib/*.css today will conflict with this requirement:

rushstack/rigs/heft-web-rig/shared/webpack-base.config.js

Lines 65 to 72 in c458420

// Handling of formats:

// - *.scss: sass, autoprefixer, modules <--- recommended

// - *.global.scss: sass, autoprefixer, NO modules

// - *.css: NO sass, NO autoprefixer, NO modules

//

// If someone wants CSS with modules, they can simply use the .sass file extension, since SASS

// is essentially a superset of CSS. (There's a small performance penalty for applying SASS to

// a plain CSS file, but the extra syntax validation probably justifies that cost.)

To handle that, we need to add a heft-sass-plugin setting that allows the file extensions to be remapped (e.g. src/*.scss -> lib/*.module.css BUT src/*.global.scss --> lib/*.global.css).

That needs to be implemented in a separate PR.

@dmichon-msft @jonasb @halfnibble

When using heft-sass-plugin to compile SASS to CSS, the mapping is:

src/**/*.scss -> lib/**/*.scss.css

src/**/*.sass -> lib/**/*.sass.css

src/**/*.css -> lib/**/*.css
This was done so that source code imports like:

import styles from './Foo.scss';

resolve correctly, whether you compile the file in advance or have webpack point at the source code. The only required change in webpack is to append ".css" to resolve.extensions.

@dmichon-msft I tried to add this, however it turns out that sass.json is not riggable because "cssOutputFolders" gets resolved relative to the rig project rather than the app project. This is a design problem with heft-sass-plugin.

We should do a separate PR to fix heft-sass-plugin, then we can follow up with heft-web-rig to enable this optimization.

rigs/heft-web-rig/shared/webpack-base.config.js

…ft-web-rig-revamp

…son "globsToDelete"

Co-authored-by: Ian Clanton-Thuon <[email protected]>

common/config/rush/nonbrowser-approved-packages.json

heft-plugins/heft-dev-cert-plugin/package.json

MickeyPhoenix · 2022-02-08T02:35:34Z

heft-plugins/heft-dev-cert-plugin/src/DevCertPlugin.ts

+// IMPORTANT: To simplify versioning, 'webpack-dev-server' is a devDependency, not a regular dependency
+import type { Configuration as WebpackDevServerConfig } from 'webpack-dev-server';


Why is it important for the reader to know, in this context, that 'webpack-dev-server' is a devDependency? I see "IMPORTANT:", and a fact -- but no explanation of why that fact is important, or what I-the-reader need to do about it.

My apologies if this question merely reflects my own ignorance, as is entirely possible.

If you try to import the value and reference it, it will compile and work locally, but fail at runtime when published. Only the type information is safe to use, hence the choice of type-only import.

@MickeyPhoenix Right, normally when you import from a package, that package must be a regular dependency (since a dev dependency will be missing for library consumers, as David explained). So the IMPORTANT note is explaining why we are doing import type.

It's a common situation for published libraries, but I can add a bit more context to the comment.

heft-plugins/heft-webpack4-plugin/package.json

MickeyPhoenix · 2022-02-08T02:58:40Z

rigs/heft-web-rig/shared/webpack-base.config.js

+            // "For production builds it's recommended to extract the CSS from your bundle being able to
+            // use parallel loading of CSS/JS resources later on. This can be achieved by using the


Should this maybe be

Suggested change

// "For production builds it's recommended to extract the CSS from your bundle being able to

// use parallel loading of CSS/JS resources later on. This can be achieved by using the

// "For production builds it's recommended to extract the CSS from your bundle so that you are

// able to use parallel loading of CSS/JS resources later on. This can be achieved by using the

or some such? I'm not sure how to parse it as it is.

The optimization is really less about parallelization and more that it lets you use the browser's optimized <link rel="stylesheet" href="..." /> hot path for importing your CSS. Also improves caching.

The quotation marks indicate that it is an excerpt from the Webpack website. The main point was that Webpack recommends to do this.

MickeyPhoenix · 2022-02-08T03:23:08Z

rigs/heft-web-rig/shared/webpack-base.config.js

+                    // Enable CSS modules...
+                    const useCssModules =
+                      // ...UNLESS the filename opts out using a file extension like "filename.global.scss"
+                      !/\.global\.\w+$/i.test(resourcePath) &&
+                      // ...UNLESS this is a .css file.
+                      !/\.css$/i.test(resourcePath);
+                    return useCssModules;


This is weird enough, maybe:

Suggested change

// Enable CSS modules...

const useCssModules =

// ...UNLESS the filename opts out using a file extension like "filename.global.scss"

!/\.global\.\w+$/i.test(resourcePath) &&

// ...UNLESS this is a .css file.

!/\.css$/i.test(resourcePath);

return useCssModules;

// Files with ".global." in the name should NOT use modules

const isGlobalFileByName = /\.global\.\w+$/i.test(resourcePath);

// Files with the ".css" extension should NOT use modules

const isCssFile = /\.css$/i.test(resourcePath);

// Use modules (i.e.return `true`) if NEITHER of the above exceptions applies

const useCssModules = !isGlobalFileByName && !isCssFile;

return useCssModules;

Why shouldn't you be allowed to write raw CSS modules? They're perfectly valid.

@halfnibble had the viewpoint that if you want to make raw CSS modules, you should use the .scss file extension. The comments in this file explain the rationale:

// If someone wants CSS with modules, they can simply use the .sass file extension, since SASS // is essentially a superset of CSS. (There's a small performance penalty for applying SASS to // a plain CSS file, but the extra syntax validation probably justifies that cost.)

So the idea is that .css should only be used for actual traditional .css files without any fancy features.

However I chatted with @iclanton and we've decided to enable autoprefixer for .css files.

MickeyPhoenix · 2022-02-08T03:28:00Z

rigs/heft-web-rig/shared/webpack-base.config.js

+                  // The "auto" setting has a confusing design:
+                  // - "false" disables CSS modules, i.e. ":local" and ":global" selectors can't be used at all
+                  // - "true" means magically disable CSS modules if the file extension isn't like ".module.css"
+                  //   or ".module.scss"
+                  // - a lambda disables CSS modules only if the lambda returns false; the function parameter is
+                  //   the resource path
+                  // - a RegExp disables CSS modules only if the resource path does not match the RegExp
+                  //
+                  // NOTE: Counterintuitively, if you instead set "modules=true" then CSS modules are enabled
+                  //       without magic, equivalent to "auto: () => true" instead of "auto: true"
+                  //
+                  // DEFAULT: "true" (i.e. path based magic)


This feels like it belongs in the css-modules documentation, not in our webpack-base.config. I suggest, instead:

Suggested change

// The "auto" setting has a confusing design:

// - "false" disables CSS modules, i.e. ":local" and ":global" selectors can't be used at all

// - "true" means magically disable CSS modules if the file extension isn't like ".module.css"

// or ".module.scss"

// - a lambda disables CSS modules only if the lambda returns false; the function parameter is

// the resource path

// - a RegExp disables CSS modules only if the resource path does not match the RegExp

//

// NOTE: Counterintuitively, if you instead set "modules=true" then CSS modules are enabled

// without magic, equivalent to "auto: () => true" instead of "auto: true"

//

// DEFAULT: "true" (i.e. path based magic)

// We define `auto` as a function so that we can selectively disable CSS modules only in the

// specific cases of (1) files with ".global." in the filename and (2) ".css" files.

// For all other files, we enable CSS modules. See the css-modules documentation for details.

This feels like it belongs in the css-modules documentation, not in our webpack-base.config.

We don't own in the css-modules project. I added these notes for posterity, to save someone else the headache of trying to understand the docs for that setting. :-)

…y bundle

… sample project to illustrate usage

…conspicuous

…d in a5f16cc

…ING)

…ass-plugin is not quite ready yet

…ft-web-rig-revamp

octogonz added 3 commits February 4, 2022 09:54

Bring over prototype of updated heft-web-rig

ba04506

Sync some changes to heft-node-rig

05a01b4

Relink workspaces dependencies

2e691b6

octogonz mentioned this pull request Feb 4, 2022

Improve heft-webpack-basic-tutorial to illustrate .scss support microsoft/rushstack-samples#3

Open

octogonz added 2 commits February 4, 2022 17:50

Sync dependencies

44827a2

rush update --full

bd7212a

octogonz commented Feb 5, 2022

View reviewed changes

rigs/heft-web-rig/shared/webpack-base.config.js Outdated Show resolved Hide resolved

octogonz commented Feb 5, 2022

View reviewed changes

rigs/heft-web-rig/shared/webpack-base.config.js Outdated Show resolved Hide resolved

octogonz commented Feb 5, 2022

View reviewed changes

rigs/heft-web-rig/shared/webpack-base.config.js Show resolved Hide resolved

octogonz commented Feb 5, 2022

View reviewed changes

octogonz added 7 commits February 4, 2022 23:40

Fix a Jest test that failed on Windows OS

fd483cc

Reenable strictPeerDependencies=true, upgrade PNPM, and solve all the…

74ec89a

… peer dependency problems

rush update --full

f0a227e

Fix up projects so everything builds successfully

99777c9

Merge remote-tracking branch 'remotes/origin/master' into octogonz/he…

9b98c7e

…ft-web-rig-revamp

rush change

4b3547e

rush change

c458420

octogonz requested review from apostolisms, D4N14L, iclanton, patmill and sachinjoseph as code owners February 5, 2022 10:39

octogonz mentioned this pull request Feb 5, 2022

[rush] Add support for rush-sdk to be webpacked. #3208

Merged

iclanton reviewed Feb 6, 2022

View reviewed changes

octogonz and others added 4 commits February 6, 2022 17:37

Merge remote-tracking branch 'remotes/origin/master' into octogonz/he…

4728061

…ft-web-rig-revamp

PR feedback -- we're not ready to release 1.0.0 yet

b00dbe3

PR feedback rush-project.json "outputFolderNames" should match heft.j…

2824e3c

…son "globsToDelete"

Update rigs/heft-web-rig/profiles/library/config/typescript.json

02e3346

Co-authored-by: Ian Clanton-Thuon <[email protected]>

MickeyPhoenix reviewed Feb 8, 2022

View reviewed changes

common/config/rush/nonbrowser-approved-packages.json Show resolved Hide resolved

MickeyPhoenix reviewed Feb 8, 2022

View reviewed changes

heft-plugins/heft-dev-cert-plugin/package.json Outdated Show resolved Hide resolved

MickeyPhoenix reviewed Feb 8, 2022

View reviewed changes

heft-plugins/heft-webpack4-plugin/package.json Outdated Show resolved Hide resolved

MickeyPhoenix reviewed Feb 8, 2022

View reviewed changes

octogonz added 13 commits February 8, 2022 19:08

Add test projects

57a632d

rush update

b70867b

Fix some issues with the Webpack config

e8f1ae8

Example app now builds an runs

a1bab5f

PR feedback - cosmetic issues

198b595

Regenerate README.md

5436f15

PR feedback -- remove @types/webpack-dev-server

770ff15

Show how to configure heft-web-rig-app-tutorial to consume the librar…

4d05712

…y bundle

PR feedback: Replace "file-loader" with asset modules, and update the…

4fed4d3

… sample project to illustrate usage

rush update

9aeadf2

Move src/templates -> src/schemas/templates to make this folder more …

1c3388d

…conspicuous

Bring back the template for sass.json, which got inadvertently delete…

f04d5f7

…d in a5f16cc

Replace sass-loader with heft-sass-plugin (work in progress, NOT WORK…

72ad161

…ING)

iclanton approved these changes Feb 11, 2022

View reviewed changes

octogonz added 6 commits February 11, 2022 01:56

Temporarily revert the sass.json features from 9aeadf2 because heft-s…

bccec15

…ass-plugin is not quite ready yet

Fix an issue where source maps weren't loaded for library projects

5ccbe25

rush change

da1d200

Merge remote-tracking branch 'remotes/origin/master' into octogonz/he…

bb0edf2

…ft-web-rig-revamp

rush update --full

06f121a

Update README.md

55ef407

octogonz merged commit 39ec31f into master Feb 11, 2022

octogonz deleted the octogonz/heft-web-rig-revamp branch February 11, 2022 10:18

iclanton mentioned this pull request Feb 15, 2022

Do not import helpers in load-themed-styles. #3234

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[heft-web-rig] Add an "app" profile, upgrade to Webpack 5, implement new CSS rules #3204

[heft-web-rig] Add an "app" profile, upgrade to Webpack 5, implement new CSS rules #3204

octogonz commented Feb 4, 2022 •

edited

Loading

octogonz Feb 5, 2022 •

edited

Loading

octogonz Feb 5, 2022

octogonz Feb 5, 2022

octogonz Feb 11, 2022

iclanton Feb 6, 2022

octogonz Feb 7, 2022

dmichon-msft Feb 7, 2022

iclanton Feb 6, 2022

octogonz Feb 11, 2022 •

edited

Loading

iclanton Feb 6, 2022

octogonz Feb 7, 2022

octogonz Feb 7, 2022

dmichon-msft Feb 7, 2022

octogonz Feb 11, 2022

MickeyPhoenix Feb 8, 2022

dmichon-msft Feb 8, 2022

octogonz Feb 9, 2022

MickeyPhoenix Feb 8, 2022

dmichon-msft Feb 8, 2022

octogonz Feb 9, 2022

MickeyPhoenix Feb 8, 2022

dmichon-msft Feb 8, 2022

octogonz Feb 9, 2022

octogonz Feb 11, 2022

MickeyPhoenix Feb 8, 2022

octogonz Feb 9, 2022

	// Handling of formats:
	// - *.scss: sass, autoprefixer, modules <--- recommended
	// - *.global.scss: sass, autoprefixer, NO modules
	// - *.css: NO sass, NO autoprefixer, NO modules
	//
	// If someone wants CSS with modules, they can simply use the .sass file extension, since SASS
	// is essentially a superset of CSS. (There's a small performance penalty for applying SASS to
	// a plain CSS file, but the extra syntax validation probably justifies that cost.)

		// IMPORTANT: To simplify versioning, 'webpack-dev-server' is a devDependency, not a regular dependency
		import type { Configuration as WebpackDevServerConfig } from 'webpack-dev-server';

		// "For production builds it's recommended to extract the CSS from your bundle being able to
		// use parallel loading of CSS/JS resources later on. This can be achieved by using the

[heft-web-rig] Add an "app" profile, upgrade to Webpack 5, implement new CSS rules #3204

[heft-web-rig] Add an "app" profile, upgrade to Webpack 5, implement new CSS rules #3204

Conversation

octogonz commented Feb 4, 2022 • edited Loading

Summary

Details

strictPeerDependencies fix

How it was tested

octogonz Feb 5, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

octogonz Feb 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

octogonz commented Feb 4, 2022 •

edited

Loading

octogonz Feb 5, 2022 •

edited

Loading

octogonz Feb 11, 2022 •

edited

Loading