STRWEB-4: Update webpack to version 5 #36

mkuklis · 2021-10-11T01:23:53Z

https://issues.folio.org/browse/STRWEB-4

There are two corresponding PRs:

stripes-cli STCLI-187: Update webpack to v5 stripes-cli#270
eslint-config-stripes ESCONF-13: Upgrade stripes-webpack to v3 and webpack to v5 eslint-config-stripes#85

These will have to be merged after this PR is merged.

Notes:

The biggest struggle with the upgrade was related the way compilation hooks were changed. The hooks object became frozen in webpack 5 (https://webpack.js.org/blog/2020-10-10-webpack-5-release/#hook-object-frozen). The way stripes webpack plugins communicate and the StripesConfigPlugin had to be refactored to follow this new pattern.

I noticed a nice performance boost from both stripes build and stripes serve. For example platform-complete builds in half of the time compared to webpack 4. The initial stripes serve (the first time stripes serve is executed in a given folder) takes similar time to what we saw in webpack 4 but the following stripes serve is about 5x faster.

The best way to try it out before this work is merged is to checkout both stripes-webpack (STRWEB-4 branch) and stripes-cli (STCLI-187 branch) into a workspace together with a single ui module and add "webpack": "^5.58.1" to resolutions to make sure the correct version is being used (this may not be required).

"resolutions": {
  "webpack": "^5.58.1",
}

Webpack 5 should open the door to a few nice features:

Better code splitting with dependsOn https://webpack.js.org/configuration/entry-context/#dependencies
Better optimization options https://webpack.js.org/configuration/optimization
Module federation (Micro frontends) @skomorokh pointed to us originally: https://webpack.js.org/concepts/module-federation

id-jenkins · 2021-10-11T01:24:50Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

zburke

Wow! So excited to see you just grabbed this sucker and went for it! 99% fine to me, but the login screen logo doesn’t load for me and I see

<img alt="..." src="[object Module]">

in the source vs

<img alt="..." src="/img/tenant-assets/opentown-libraries-logo.c96ff678691e1a345321b50941335d81.png">

from folio-snapshot. I see the same behavior whether in a workspace or platform build. Trying to compile a build to an output directory, I get

Module not found: Error: Can't resolve 'postcss-loader' in '/Users/zburke/temp/platform-core'

@JohnC-80, do you recall if there were any webpack-5 specific notes in the post-css work (#31) you did a few weeks ago that now come into play?

These are the only two glitches I've found so far. Nice work!

Edit: additional config details:

$ yarn --version
1.22.11
$ node --version
v16.10.0
platform-core(snapshot)*
$ npm --version
7.24.0
platform-core(snapshot)*
$ uname -a
Darwin LP-C02G8208ML85 20.6.0 Darwin Kernel Version 20.6.0: Mon Aug 30 06:12:21 PDT 2021; root:xnu-7195.141.6~3/RELEASE_X86_64 x86_64

mkuklis · 2021-10-13T18:35:28Z

Thanks for trying this out @zburke!

Would you mind sharing your workspace setup with me?

I will try to checkout stripes-cli#STCLI-187 and stripes-webpack#STRWEB-4 with a single ui module (maybe ui-users) in order to reproduce it.

Would that be enough?

zburke · 2021-10-13T19:05:31Z

@mkuklis, a straight clone of platform-core#snapshot (i.e. not a workspace) with the following resolutions shows both glitches:

  "@folio/stripes-cli": "folio-org/stripes-cli#STCLI-187",
  "@folio/stripes-webpack": "folio-org/stripes-webpack#STRWEB-4",

when serving via yarn local or building via yarn stripes build stripes.config.js.

JohnC-80 · 2021-10-13T19:28:18Z

Yeah, @mkuklis - Thank you for grabbing this bull by the horns. I'm very interested in what federated modules will ultimately be able to do for us.
Not gonna lie, I'm a little jealous of what you may have learned in this process.
I think we'll ultimately want to upgrade all of the loaders to their latest version that have probably shuffled off backwards compatibility with webpack v4...
postcss-loader, css-loader are on this 'definitely' list in my mind, happy to take on those upgrades.

There's been some weirdness with the branding icons in a workspace... I haven't had a chance to look into it yet. The wrong thing getting stringified/stringifying the object when it probably needs to use a key of the object (I suspect).

id-jenkins · 2021-10-14T03:07:39Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

mkuklis · 2021-10-14T03:20:02Z

Thank you for your feedback @JohnC-80. You are exactly right! I had to update postcss-loader. The version we had there was not compatible with webpack 5 anymore.

There is also css-loader which can be updated too but we will first need to migrate to asset modules https://webpack.js.org/guides/asset-modules before we can update css-loader. The current version works with webpack 5 so I'm going to leave it for now.

@zburke I would appreciate if you could give it another try with the most recent changes.

Oh I also had to upgrade eslint-config-stripes: folio-org/eslint-config-stripes#85

zburke

With postcss v8 et al in place, images are now rendering correctly for me. Impossible to overstate how great this news is! LGTM! 🎉 🎉 🎉

mkuklis · 2021-10-14T13:04:44Z

Thank you for your help with testing this @zburke!

zburke · 2021-10-14T16:00:07Z

Looks like maaaaybe some CSS stuff still isn't coming through correctly? ~~Note also "form.restart is not a function". I have no idea where that's coming from 🤔 .~~ **

My localhost is from a workspace with a platform and some stripes libs cloned (all UI apps are just pulled in via the platform). Compare with the folio-snapshot:

** form.restart is a red-herring from an out-dated final-form dep; form.restart was added in v4.20.x but my build had only v4.18.x. Er, it's a red-herring WRT this PR, but that dep does need to get updated elsewhere.

mkuklis · 2021-10-14T18:48:34Z

Good find @zburke! ~~I wonder what that form.restart is :)~~
I will take a look at these css issues.

zburke · 2021-10-14T19:30:38Z

@mkuklis, form.restart is a red-herring from an out-dated final-form dep. It's part of v4.20.x but my build had only v4.18.x. That's still a problem, but at least it's not a problem with this PR :)

id-jenkins · 2021-10-15T01:56:44Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

mkuklis · 2021-10-15T01:56:47Z

@zburke it looks like the issue with colors is related to https://github.com/postcss/postcss-color-function which has been deprecated. There is a PR there to update postcss version postcss/postcss-color-function#60 but it hasn't been merged. I pointed to that fork in stripes-webpack and it looks like that solved the problem for now.

There is also a newer lib (by the same author) which changes color function to color-mod:
https://github.com/csstools/postcss-color-mod-function

That lib is maintained and up to date but it looks like color-mod is also not part of the css spec anymore.

If we want to move to https://github.com/csstools/postcss-color-mod-function stripes-components would have to be updated to use color-mod() instead of color().

There are couple places where color() is currently used in stripes-components:

./lib/Card/Card.css
./lib/variables.css
./lib/Datepicker/Calendar.css
./lib/Callout/Callout.css
./lib/Button/Button.css
./lib/sharedStyles/form.css

I think since this was removed from the css spec perhaps a better alternative would be to refactor these to not use color() or color-mod() at all. I will let @JohnC-80 decide what to do here.

For now I'm leaving postcss-color-function (from the fork) so we can move forward with the webpack upgrade.

@zburke thanks again for catching this!

id-jenkins · 2021-10-15T03:41:58Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

zburke · 2021-10-15T13:14:35Z

package.json

-    "postcss-url": "^8.0.0",
+    "postcss": "^8.3.9",
+    "postcss-calc": "^8.0.0",
+    "postcss-color-function": "onigoetz/postcss-color-function",


For now I'm leaving postcss-color-function (from the fork) so we can move forward with the webpack upgrade.

Man, it feels like there just aren't good options here. We have some old (now incompatible) things that are unmaintained/deprecated but actually published, and we have some new forks that provide compatibility but haven't ever been formally published. Using a GitHub fork is a bit nerve-wracking. It's relatively hard to unpublish something from NPM, but it's super easy to remove a repository from GitHub.

Yeah this is just a temporary solution until stripes-components is cleaned up. If you are worry about the fork we can fork it too for now.

But I agree. I think the best would be to take care of the color right in stripes-components. Then we could remove postcss-color-function completely.

@mkuklis, shall I write up an STCOM ticket (and corresponding STRWEB ticket) for this to make sure it gets handled for Lotus/R1-2022? @JohnC-80, do you foresee any reason to believe it would be a problem getting such work into Louts?

@zburke that sounds good to me. I cloned postcss-color-function to folio-org just in case the other repo disappears :).

id-jenkins · 2021-10-18T12:26:16Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

id-jenkins · 2021-10-18T16:06:24Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

id-jenkins · 2021-11-02T15:41:59Z

yarn run v1.22.5
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

This reverts commit ac40b57, reversing changes made to b0068bd.

* Revert "STRWEB-4 add crypto-browserify depenency to polyfill crypto in axe test runs (#37)" This reverts commit ec148a4. * Revert "Merge pull request #36 from folio-org/STRWEB-4" This reverts commit ac40b57, reversing changes made to b0068bd.

Update webpack to version 5

348d006

mkuklis mentioned this pull request Oct 11, 2021

STCLI-187: Update webpack to v5 folio-org/stripes-cli#270

Merged

mkuklis requested review from zburke, adi-mat, MikeTaylor, skomorokh and ryandberger October 11, 2021 12:40

zburke requested changes Oct 13, 2021

View reviewed changes

Update loaders

eb16062

mkuklis mentioned this pull request Oct 14, 2021

ESCONF-13: Upgrade stripes-webpack to v3 and webpack to v5 folio-org/eslint-config-stripes#85

Merged

mkuklis requested a review from zburke October 14, 2021 03:20

zburke approved these changes Oct 14, 2021

View reviewed changes

mkuklis requested a review from axelhunn October 14, 2021 15:00

Point to an updated version of postcss-color-function

841fe40

Update css-loader

5ab2197

zburke reviewed Oct 15, 2021

View reviewed changes

Move webpack to peer and dev dependencies

184b508

Point postcss-color-function to folio-org

c47b3c4

zburke mentioned this pull request Oct 28, 2021

STCOM-892 upgrade postcss to v8, webpack to v5 folio-org/stripes-components#1642

Merged

add node utility shims for testing

96e26f1

ryandberger approved these changes Nov 4, 2021

View reviewed changes

ryandberger merged commit ac40b57 into master Nov 4, 2021

mkuklis added a commit that referenced this pull request Nov 5, 2021

Revert "Merge pull request #36 from folio-org/STRWEB-4"

6a94b84

This reverts commit ac40b57, reversing changes made to b0068bd.

zburke deleted the STRWEB-4 branch January 21, 2022 16:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STRWEB-4: Update webpack to version 5 #36

STRWEB-4: Update webpack to version 5 #36

mkuklis commented Oct 11, 2021 •

edited

Loading

id-jenkins commented Oct 11, 2021

zburke left a comment •

edited

Loading

mkuklis commented Oct 13, 2021 •

edited

Loading

zburke commented Oct 13, 2021

JohnC-80 commented Oct 13, 2021

id-jenkins commented Oct 14, 2021

mkuklis commented Oct 14, 2021

zburke left a comment

mkuklis commented Oct 14, 2021

zburke commented Oct 14, 2021 •

edited

Loading

mkuklis commented Oct 14, 2021 •

edited

Loading

zburke commented Oct 14, 2021

id-jenkins commented Oct 15, 2021

mkuklis commented Oct 15, 2021 •

edited

Loading

id-jenkins commented Oct 15, 2021

zburke Oct 15, 2021

mkuklis Oct 15, 2021

mkuklis Oct 15, 2021

zburke Oct 18, 2021

mkuklis Oct 18, 2021

id-jenkins commented Oct 18, 2021

id-jenkins commented Oct 18, 2021

id-jenkins commented Nov 2, 2021

STRWEB-4: Update webpack to version 5 #36

STRWEB-4: Update webpack to version 5 #36

Conversation

mkuklis commented Oct 11, 2021 • edited Loading

Notes:

id-jenkins commented Oct 11, 2021

zburke left a comment • edited Loading

Choose a reason for hiding this comment

mkuklis commented Oct 13, 2021 • edited Loading

zburke commented Oct 13, 2021

JohnC-80 commented Oct 13, 2021

id-jenkins commented Oct 14, 2021

mkuklis commented Oct 14, 2021

zburke left a comment

Choose a reason for hiding this comment

mkuklis commented Oct 14, 2021

zburke commented Oct 14, 2021 • edited Loading

mkuklis commented Oct 14, 2021 • edited Loading

zburke commented Oct 14, 2021

id-jenkins commented Oct 15, 2021

mkuklis commented Oct 15, 2021 • edited Loading

id-jenkins commented Oct 15, 2021

zburke Oct 15, 2021

Choose a reason for hiding this comment

mkuklis Oct 15, 2021

Choose a reason for hiding this comment

mkuklis Oct 15, 2021

Choose a reason for hiding this comment

zburke Oct 18, 2021

Choose a reason for hiding this comment

mkuklis Oct 18, 2021

Choose a reason for hiding this comment

id-jenkins commented Oct 18, 2021

id-jenkins commented Oct 18, 2021

id-jenkins commented Nov 2, 2021

mkuklis commented Oct 11, 2021 •

edited

Loading

zburke left a comment •

edited

Loading

mkuklis commented Oct 13, 2021 •

edited

Loading

zburke commented Oct 14, 2021 •

edited

Loading

mkuklis commented Oct 14, 2021 •

edited

Loading

mkuklis commented Oct 15, 2021 •

edited

Loading