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 npm dependencies in CHT Conf - post-Node >= 18 #621

Closed
m5r opened this issue Jul 8, 2024 · 4 comments · Fixed by #628
Closed

Update npm dependencies in CHT Conf - post-Node >= 18 #621

m5r opened this issue Jul 8, 2024 · 4 comments · Fixed by #628
Assignees
Labels
Type: Technical issue Improve something that users won't notice

Comments

@m5r
Copy link
Member

m5r commented Jul 8, 2024

Describe the issue
Last time we updated dependencies we were still supporting node 8. Now that we've dropped support for node versions < 18, we can move forward and update the dependencies we couldn't update before.

Describe the improvement you'd like
Update npm dependencies and test to make sure nothing is broken after the updates.
We are behind many major versions of most packages we depend on, some of them are no longer maintained and need replacing like eslint-loader and eslint-plugin-node.

I made an attempt at upgrading ESLint to allow it to accept more recent syntax but it led me to replace eslint-loader with eslint-webpack-plugin, replace eslint-plugin-node with eslint-plugin-n, upgrade webpack to version 5, and then I eventually ran out of time to focus on the issue I was initially dealing with 🤷‍♂️

One important thing to note is that since cht-conf v3, CHT deployments configurations have their own ESLint configuration (documented here). ESLint v8 deprecates the json config file approach .eslintrc for a javascript config file eslint.config.js instead. With ESLint v9, they dropped support for it and it no longer works without migrating to the new config format. They provide a CLI that automates some of the work for you but it still needs some manual tweaks to make it work for each project. This represents a breaking change for app devs that needs to be well documented and means a cht-conf v5.
For now I would suggest updating ESLint to v8 to support both formats for now and update to v9 (or latest if v9 becomes obsolete) at a later time, when we're comfortable with releasing a cht-conf v5.
EDIT: feedback from a team who recently migrated to ESLint v9 with Flat Config:

I expect similar occurrences with some of our dependencies, ESM/CJS incompatibilities with some others of our dependencies. Node 8.0 was released in 2017 (with the last version 8.17 in 2019) and the ecosystem has evolved a lot in that timeframe, we have 10 major versions of Node to catch up on and I estimate this maintenance to take quite some time (along the lines of weeks, not months - but definitely not days). IMO the part of the code that need extra attention during this process is the compilation code in src/lib/compilation/*.js that compiles the contact summary, tasks, and targets when compiling app settings.

@m5r m5r added the Type: Technical issue Improve something that users won't notice label Jul 8, 2024
@garethbowen
Copy link
Member

Ideally we'd have the e2e testing suite merged before embarking on this to give us more confidence that nothing has regressed.

@m5r
Copy link
Member Author

m5r commented Jul 17, 2024

I opened PR #623 to update non-breaking dependencies. That leaves us with those dependencies left to upgrade:

  ◉ chai                            ^4.3.10  →    ^5.1.1
  ◉ chai-as-promised                 ^7.1.1  →    ^8.0.0
  ◉ chai-exclude                     ^2.1.0  →    ^2.1.1
  ◉ eslint                           ^6.8.0  →    ^9.7.0
  ◉ googleapis                      ^84.0.0  →  ^140.0.1
  ◉ open                             ^8.4.2  →   ^10.1.0
  ◉ pouchdb-adapter-http             ^7.2.2  →    ^9.0.0
  ◉ pouchdb-adapter-memory           ^7.2.2  →    ^9.0.0
  ◉ pouchdb-core                     ^7.2.2  →    ^9.0.0
  ◉ pouchdb-mapreduce                ^7.2.2  →    ^9.0.0
  ◉ pouchdb-session-authentication   ^1.3.0  →    ^1.4.0
  ◉ semantic-release                ^22.0.0  →   ^24.0.0
  ◉ terser-webpack-plugin            ^1.4.3  →   ^5.3.10
  ◉ webpack                         ^4.46.0  →   ^5.93.0
  ◉ xpath                            0.0.33  →    0.0.34

This list doesn't include the deprecated/unmaintained dependencies we might have to replace. WebStorm calls out request as vulnerable and its readme makes note that the package is deprecated and points to this issue for potential alternatives.

Like mentioned in the PR description we're:

  • keeping semantic-release on v22 instead of updating to latest v24 because starting with v23 they stopped supporting Node.js 18
  • not updating chai because chai-exclude still relies on chai <= 4 in their peerDependencies - note that this is changing, they recently merged a PR that resolves this, so we can expect a release soon allowing us to update chai then EDIT: see next item
  • keeping chai, chai-as-promised, and chai-exclude respectively on v4.3.10, v7.1.1, v2.1.0 because starting with their next major version they become ESM-only packages
  • keeping open on v8.4.2 instead of updating to latest v10 because starting with v9 it's an ESM-only package and ESLint is so outdated it can't parse the await import('package') syntax 🤦‍♂️
  • not updating xpath from v0.0.33 to v0.0.34 because no changelog is provided, I'm not sure what changed in that version and this is an important dependency for parsing XPath paths in forms
  • keeping @parcel/watcher on v2.1.0 because v2.2.0-2.4.0 have incompatibilities with npm 10.4 (see gyp: binding.gyp not found (...) while trying to load binding.gyp parcel-bundler/watcher#156) and v2.4.1 has a weird bug where the watcher hangs ~5% of the time when unsubscribing (see chore(#621): update non-breaking dependencies #623 (comment))

I'm not sure yet whether updating pouchdb-* dependencies will have to wait until we also update it in cht-core, I haven't looked into it yet.

My next initiative will be to update eslint to v8 to keep compatibility with our current eslint config format allowing us to give app developers and ourselves enough time to migrate to the new eslint config format before updating to v9.

m5r added a commit that referenced this issue Jul 22, 2024
* upgrade non-breaking dev dependencies

* why rewire...

* forgot .only

* upgrade prod dependencies

* silence grandpa eslint who cannot resolve a `require`

* not sure why test is passing locally but failing in CI

* .only

* debug CI

* debug CI - try force color in diff

* debug CI colors

* debug CI colors

* okay that fixed it, revert test back and remove logs

* .only

* remove unnecessary `{ color: true }` parameter

* comment eslint ignore

* sonar

* will this space make sonar happy?

* will this?

* this should.

* clamp `@parcel/watcher` to v2.1.0 because more recent versions either have incompatibilities with npm 10.4 (see parcel-bundler/watcher#156) or the watcher hangs ~5% of the time when unsubscribing (see #623 (comment))

* remove todos
m5r added a commit that referenced this issue Jul 22, 2024
* update eslint to v8

* make our current webpack setup work with eslint 8

* remove TODOs now that eslint knows how to parse this syntax

* merge package-lock.json

* bump `es6` => `es2022` in eslint config
@m5r
Copy link
Member Author

m5r commented Jul 22, 2024

Tackling the webpack upgrade, I'm running into same issue Gareth ran into when trying to update webpack a while ago. Our compile-contact-summary.js tests are failing because the compilation hangs indefinitely. I haven't found the cause yet but I'm building a minimal repro case to replicate the bug and locate where it's coming from.

m5r added a commit that referenced this issue Jul 24, 2024
* upgrade `webpack` and `terser-webpack-plugin`

* use `xxhash64` hash function (see #540 (review))

* replace the deprecated `webpack.IgnorePlugin` constructor API with the new one

* warning is now an object instead of a string

* I can't believe it took me 2 days to figure this out 🤦‍♂️

* simplify webpack config
@m5r m5r linked a pull request Jul 24, 2024 that will close this issue
m5r added a commit that referenced this issue Jul 29, 2024
* update to latest the non-breaking deps that received new releases since last week

* update `googleapis`
@m5r m5r closed this as completed in #628 Jul 29, 2024
@m5r
Copy link
Member Author

m5r commented Jul 29, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Technical issue Improve something that users won't notice
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants