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 webpack to v5.xx #1258

Merged
merged 4 commits into from
Jun 16, 2021
Merged

Update webpack to v5.xx #1258

merged 4 commits into from
Jun 16, 2021

Conversation

kr8n3r
Copy link
Contributor

@kr8n3r kr8n3r commented Mar 9, 2021

What

Update webpack to latest version along with the config update, plugin updates and mitigation of any deprecations

Co-authored with @0atman and @paroxp

Build comparison with v4

Development

V4

Version: webpack 4.46.0
Time: 46367ms

assets/48v2kvrekp35frscdv7ven58sz.govuk.screen.css.gz   133 KiB                 [emitted] [immutable]         
assets/4gcsjp48bgbzzsszhckxm26zcr.s3.png  12.5 KiB                 [emitted] [immutable]         
assets/52p52jew9y51hxvdq1utja7snm.sqs.png  26.5 KiB                 [emitted] [immutable]         
assets/593uuprq7mbtftfpnr8n5jga1t.elasticsearch.png  24.5 KiB                 [emitted] [immutable]         
assets/6dqyq926rhx38q757cu216ab3k.redis.png  12.5 KiB                 [emitted] [immutable]         
assets/6t1fdd961jkt8ak9fsvabzy2.autoscaler.png  13.6 KiB                 [emitted] [immutable]         
assets/6u724h1ubdx4zf7dp9993hx5.mysql.png  3.67 KiB                 [emitted] [immutable]         
assets/init.js.gz  4.72 KiB                 [emitted]                     
assets/init.js.map  18.7 KiB    assets/init  [emitted] [dev]               assets/init
assets/sankey.js.gz  2.09 KiB                 [emitted]                     
assets/sankey.js.map  7.35 KiB  assets/sankey  [emitted] [dev]               assets/sankey
assets/u89au5uepx75kcgbpnjc6b2vb.cdn.png   9.8 KiB                 [emitted] [immutable]         
main.js   815 KiB           main  [emitted]              [big]  main
main.js.map  1.14 MiB           main  [emitted] [dev]               main
Entrypoint main [big] = main.js main.js.map
Entrypoint assets/init = assets/init.js assets/init.js.map
Entrypoint assets/sankey = assets/sankey.js assets/sankey.js.map

V5

assets by chunk 460 KiB (auxiliary name: main) 11 assets
assets by path assets/*.gz 21.7 KiB
  asset assets/govuk.screen.css.gz 16.1 KiB [emitted] [compressed]
  asset assets/init.js.gz 4.47 KiB [emitted] [compressed]
  asset assets/sankey.js.gz 1.13 KiB [emitted] [compressed]
asset main.js 807 KiB [emitted] [big] (name: main) 1 related asset
asset assets/init.js.map 17.1 KiB [emitted] [dev] (auxiliary name: init)
asset assets/sankey.js.map 3.95 KiB [emitted] [dev] (auxiliary name: sankey)
Entrypoint main [big] 807 KiB (1.58 MiB) = main.js 12 auxiliary assets
Entrypoint init (17.1 KiB) = 1 auxiliary asset
Entrypoint sankey (3.95 KiB) = 1 auxiliary asset
webpack 5.38.1 compiled successfully in 35413 ms

Production

V4:

Version: webpack 4.46.0
Time: 43854ms
assets/3g9yb8tbkf62xq3sf7ucu3472d.govuk.screen.css.gz  14.6 KiB          [emitted] [immutable]         
assets/init.js.gz  2.75 KiB          [emitted]                     
assets/init.js.map  25.4 KiB       0  [emitted] [dev]               assets/init
assets/sankey.js.gz  1.28 KiB          [emitted]                     
assets/sankey.js.map  9.89 KiB       1  [emitted] [dev]               assets/sankey
assets/u89au5uepx75kcgbpnjc6b2vb.cdn.png   9.8 KiB          [emitted] [immutable]         
main.js   441 KiB       2  [emitted]              [big]  main
main.js.map  1010 KiB       2  [emitted] [dev]               main
Entrypoint main [big] = main.js main.js.map
Entrypoint assets/init = assets/init.js assets/init.js.map
Entrypoint assets/sankey = assets/sankey.js assets/sankey.js.map

V5

assets by path assets/*.png 215 KiB 10 assets
assets by path assets/*.gz 19 KiB
  asset assets/govuk.screen.css.gz 14.3 KiB [emitted] [compressed]
  asset assets/init.js.gz 3.62 KiB [emitted] [compressed]
  asset assets/sankey.js.gz 1.07 KiB [emitted] [compressed]
asset main.js 762 KiB [emitted] [big] (name: main)
Entrypoint main [big] 762 KiB (215 KiB) = main.js 10 auxiliary assets
Entrypoint init =
Entrypoint sankey =
webpack 5.38.1 compiled successfully in 40905 ms

How to review

  • run locally or deploy to dev env and check app works
  • review code

Who can review

anyone

@kr8n3r
Copy link
Contributor Author

kr8n3r commented Apr 30, 2021

@0atman
Copy link
Contributor

0atman commented May 20, 2021

Merge successful

Test Suites: 79 passed, 79 total
Tests:       756 passed, 756 total
Snapshots:   0 total
Time:        14.779 s, estimated 30 s
Ran all test suites.

@0atman
Copy link
Contributor

0atman commented May 20, 2021

node-external added

Tests:       756 passed, 756 total

@0atman
Copy link
Contributor

0atman commented May 20, 2021

I've ensured that package.json now adheres to all recommendations here https://webpack.js.org/migrate/5/

config/webpack.config.js Outdated Show resolved Hide resolved
config/webpack.config.js Outdated Show resolved Hide resolved
@0atman
Copy link
Contributor

0atman commented May 27, 2021

the error does indeed point to a font in the frontend:

λ ack light-94a07e06a1-v2.woff2
govuk/helpers/_font-faces.scss
24:            govuk-font-url("light-94a07e06a1-v2.woff2") format("woff2")

@0atman 0atman marked this pull request as ready for review June 2, 2021 13:31
@0atman 0atman self-assigned this Jun 2, 2021
@0atman
Copy link
Contributor

0atman commented Jun 2, 2021

@kr8n3r I can't keep up with the PRs 😆 this is ready to review!

@0atman 0atman changed the title WIP: Update webpack to v5.xx Update webpack to v5.xx Jun 2, 2021
@0atman
Copy link
Contributor

0atman commented Jun 2, 2021

conflict in merged lock:

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! Found: webpack@4.46.0
npm ERR! node_modules/webpack
npm ERR!   dev webpack@"^4.46.0" from the root project
npm ERR!   peer webpack@"4.x.x || 5.x.x" from @webpack-cli/configtest@1.0.3
npm ERR!   node_modules/@webpack-cli/configtest
npm ERR!     @webpack-cli/configtest@"^1.0.3" from webpack-cli@4.7.0
npm ERR!     node_modules/webpack-cli
npm ERR!       dev webpack-cli@"^4.7.0" from the root project
npm ERR!       3 more (@webpack-cli/configtest, @webpack-cli/info, @webpack-cli/serve)
npm ERR!   6 more (css-loader, mini-css-extract-plugin, ...)
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer webpack@"^5.1.0" from compression-webpack-plugin@7.1.2
npm ERR! node_modules/compression-webpack-plugin
npm ERR!   dev compression-webpack-plugin@"^7.1.2" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 
npm ERR! See /home/oatman/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/oatman/.npm/_logs/2021-06-02T15_17_27_165Z-debug.log

@kr8n3r
Copy link
Contributor Author

kr8n3r commented Jun 3, 2021

  • in dev env css path/filename is undefined - resolved in 4c0826d

pushed up rewritten commit history to make rebasing easier

As mini-css-extract-plugin ties to insert a link tag (via document.) it doesn't really work with SSR.
As a workaround we specify the output filename and reference that name in the template - but we're loosing out on hashing

Need to figure out if theres a way be can export [contenthash:base32]] from the build and import it into the template to append to the path

@kr8n3r kr8n3r force-pushed the update-webpack branch 8 times, most recently from 476e892 to a7474bb Compare June 4, 2021 11:17
This was referenced Jun 8, 2021
@0atman
Copy link
Contributor

0atman commented Jun 9, 2021

Riddle me this:

ERROR in ./src/layouts/template.tsx 7:43-73
Module not found: Error: Can't resolve '../../dist/manifest' in '/home/oatman/projects/gds/paas/paas-admin/src/layouts'resolve '../../dist/manifest' in '/home/oatman/projects/gds/paas/paas-admin/src/layouts'
  using description file: /home/oatman/projects/gds/paas/paas-admin/package.json (relative path: ./src/layouts)
    using description file: /home/oatman/projects/gds/paas/paas-admin/package.json (relative path: ./dist/manifest)
      no extension
        /home/oatman/projects/gds/paas/paas-admin/dist/manifest doesn't exist
      .ts
        /home/oatman/projects/gds/paas/paas-admin/dist/manifest.ts doesn't exist
      .tsx
        /home/oatman/projects/gds/paas/paas-admin/dist/manifest.tsx doesn't exist
      .js
        /home/oatman/projects/gds/paas/paas-admin/dist/manifest.js doesn't exist
      .json
        /home/oatman/projects/gds/paas/paas-admin/dist/manifest.json doesn't exist
      as directory
        /home/oatman/projects/gds/paas/paas-admin/dist/manifest doesn't exist
 @ ./src/layouts/index.ts 6:21-42
 @ ./src/components/application-events/controllers.tsx 8:18-42
 @ ./src/components/application-events/index.ts 4:21-45
 @ ./src/components/app/router.ts 7:47-79
 @ ./src/components/app/index.ts 10:21-40
 @ ./src/main.ts 6:38-65

webpack 5.38.1 compiled with 1 error in 22821 ms

AND YET

λ cat /home/oatman/projects/gds/paas/paas-admin/dist/manifest.json
{
  "main.js": "/main.js",
  "assets/govuk.screen.css.map": "/assets/6e483ced48ac3951527egovuk.screen.css.map",
  "assets/postgres.png": "/assets/1b9b2da8d94b8c3d4a15.postgres.png",
  "assets/influxdb.png": "/assets/edd08553c3a7de815b44.influxdb.png",
  "assets/sqs.png": "/assets/93629362d6c0b2dd2120.sqs.png",
  "assets/elasticsearch.png": "/assets/19a417093d9756975d56.elasticsearch.png",
  "assets/init.js.map": "/assets/init.js.map",
  "assets/govuk.screen.css.gz": "/assets/6e483ced48ac3951527egovuk.screen.css.gz",
  "assets/autoscaler.png": "/assets/c17f95367648a6936504.autoscaler.png",
  "assets/redis.png": "/assets/52a85440d0cb10638fe8.redis.png",
  "assets/s3.png": "/assets/77fd1266970b7e8cff57.s3.png",
  "assets/cdn.png": "/assets/6a075556a455bd2589e9.cdn.png",
  "assets/cloud.png": "/assets/e68566b16e48eaf1308b.cloud.png",
  "assets/init.js.gz": "/assets/init.js.gz",
  "assets/sankey.js.map": "/assets/sankey.js.map",
  "assets/mysql.png": "/assets/a4438110aaccb83f3a53.mysql.png",
  "assets/sankey.js.gz": "/assets/sankey.js.gz",
  "main.js.map": "/main.js.map"
}                                                                                                                   

src/layouts/template.tsx Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@0atman
Copy link
Contributor

0atman commented Jun 9, 2021

Progress, faking the manifest until after build pushes the import error into after tests pass:

Test Suites: 79 passed, 79 total
Tests:       757 passed, 757 total
Snapshots:   0 total
Time:        15.034 s, estimated 31 s
Ran all test suites.
error: Unable to resolve path to module '../../dist/manifest' (import/no-unresolved) at src/layouts/template.tsx:4:22:
  2 | import { renderToStaticMarkup } from 'react-dom/server';
  3 | 
> 4 | import manifest from '../../dist/manifest';
    |                      ^
  5 | import { IViewContext } from '../components/app';
  6 | import { Breadcrumbs, IBreadcrumbsItem } from '../components/breadcrumbs';
  7 | 

@0atman
Copy link
Contributor

0atman commented Jun 9, 2021

Nasty hacks, we got'em. But the damn thing works.

Screenshot from 2021-06-09 14-04-52

@0atman
Copy link
Contributor

0atman commented Jun 9, 2021

Ready to review @kr8n3r and @paroxp et al. Please look at the Cleanup Actions section in the description for tasks I'd like advice on ☝️

@0atman
Copy link
Contributor

0atman commented Jun 9, 2021

Even with the watcher disabled, the builder cycles building between TS and the manifest.json. I think we have a circular dependency.
the assets are built, their hashes are recorded in the manifest, but then it's imported into the TS code - which then invalidates that asset, meaning the TS gets rebuilt etc etc

asset manifest.json 1.06 KiB [emitted]
Entrypoint main [big] 808 KiB (1.59 MiB) = main.js 12 auxiliary assets
Entrypoint init (17.1 KiB) = 1 auxiliary asset
Entrypoint sankey (3.95 KiB) = 1 auxiliary asset
webpack 5.38.1 compiled successfully in 813 ms
assets by status 1.28 MiB [cached] 17 assets
asset manifest.json 1.06 KiB [emitted]
Entrypoint main [big] 808 KiB (1.59 MiB) = main.js 12 auxiliary assets
Entrypoint init (17.1 KiB) = 1 auxiliary asset
Entrypoint sankey (3.95 KiB) = 1 auxiliary asset
webpack 5.38.1 compiled successfully in 818 ms
assets by status 1.28 MiB [cached] 17 assets
asset manifest.json 1.06 KiB [emitted]
Entrypoint main [big] 808 KiB (1.59 MiB) = main.js 12 auxiliary assets
Entrypoint init (17.1 KiB) = 1 auxiliary asset
Entrypoint sankey (3.95 KiB) = 1 auxiliary asset
webpack 5.38.1 compiled successfully in 822 ms
assets by status 1.28 MiB [cached] 17 assets
asset manifest.json 1.06 KiB [emitted]
Entrypoint main [big] 808 KiB (1.59 MiB) = main.js 12 auxiliary assets
Entrypoint init (17.1 KiB) = 1 auxiliary asset
Entrypoint sankey (3.95 KiB) = 1 auxiliary asset
webpack 5.38.1 compiled successfully in 871 m```

@0atman
Copy link
Contributor

0atman commented Jun 9, 2021

After trying and faling to disable the webpack watcher and rebuilding processes, I think a solution to this, perhaps, is to disable watching in dev, and wrap the whole webpack build in a different watching process, one that is outside of the webpack dependency world, perhaps something like https://www.npmjs.com/package/npm-watch

@kr8n3r
Copy link
Contributor Author

kr8n3r commented Jun 10, 2021

I have looked at other ways of sorting this circular dependency rebuilt issue and have ultimately decided to revert back to un-hashed css file name. More details in e7d7943

@kr8n3r kr8n3r force-pushed the update-webpack branch 2 times, most recently from 37c77ef to 0c8eeb5 Compare June 14, 2021 08:06
@0atman
Copy link
Contributor

0atman commented Jun 14, 2021

I'm up-to-date. Sounds totally reasonable to go back to unhashed filenames, given the trouble we had.

@kr8n3r kr8n3r force-pushed the update-webpack branch 2 times, most recently from 0d086c5 to 8ac2669 Compare June 15, 2021 13:34
kr8n3r and others added 4 commits June 15, 2021 16:02
Bump webpack to v5.x and required plugin versions
- Removed `output.path: path.resolve(__dirname, 'dist')` as that's the
default.

- Removed `output.filename: '[name].js'` as that's the default.

- Replaced `[hash]` with `[contenthash]` - It is not the same, but proven
to be more effective - from migration docs

- Update entry and devtool configuration
I don't think there's a need to have the source maps in production,
especially as init.js filesize increases to +80KB

Webpack 5 seems not like to generate sourcemaps with the old entry
naming. Switching to [Entry Descriptor](https://webpack.js.org/configuration/entry-context/#entry-descriptor)
fixes that.

- Bump compression-webpack-plugin and set it to keep sourcemaps

- Use asset modules for static assets
`file-loader` is deprected for v5 and https://webpack.js.org/guides/asset-modules/
is the recomended approach.

- Use mini-css-extract-plugin to generate a stylesheet

- Remove @intervolga/optimize-cssnano-plugi andreplace with
CssMinimizerPlugin
- update css extract plugin config with filename
- update css import and reference in the template file

Mini-css-extract-plugin generates a file and wan'ts to insert the link
to the assets in the browser.
This being a server-side rendered app, it cannot do that, so we need to
manually link to the file.

While webpack can gerate the file with a cache-busting string there is
no reliable way to export that and use in the template.
We have tried with webpack-manifest plugin, but ran into a
circular-dependency contstant rebuild and a test error for an empty file.

While ideally we'd have cache-busting, but our server is already set
without any cache headers (suspect because of client js), so the broser
will request the CSS file everytime, meaning we're not loosing much if
anything.
Happy to accept this compromise until we can spend more time looking at
the whole pipeline.
Copy link
Member

@paroxp paroxp left a comment

Choose a reason for hiding this comment

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

Let's just get it over with, I cannot stand looking at this card no mo...

Good job everyone.

@0atman
Copy link
Contributor

0atman commented Jun 16, 2021

Same!

@paroxp paroxp merged commit ffb9158 into main Jun 16, 2021
@paroxp paroxp deleted the update-webpack branch June 16, 2021 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file major version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants