Node.js upgrade to v18 along with dependency upgrades and linting fixes #645

bjester · 2024-05-22T21:12:40Z

Description

Upgrades Node.js to v18, along with Kolibri-Tools to v0.16, Jest to v29 and Nuxt to v2.15. Additionally, this pins dependencies in order to maintain compatibility between Nuxt and kolibri-tools, applies fixes for new linting errors, and cleans up testing output through better error handling.

Issue addressed

Addresses #439

Changelog

[Node.js upgrade to v18 along with dependency upgrades and linting fixes #645]
- Description: Upgrades Node.js to v18, along with Kolibri-Tools to v0.16, Jest to v29 and Nuxt to v2.15
- Products impact: none
- Addresses: Upgrade to NodeJS 18 #439
- Components: none
- Breaking: no
- Impacts a11y: no
- Guidance: Netlify configuration needs to be updated to use v18 as well
[Node.js upgrade to v18 along with dependency upgrades and linting fixes #645]
- Description: Component error handling now use @error listener to avoid polluting test output, nor suppressing console.* in tests
- Products impact: any
- Addresses: n/a
- Components: KImg, KTabs, KTabsList
- Breaking: yes
- Impacts a11y: no
- Guidance: The KImg component may now emit an Error object in its @error listener when incorrectly configured, in addition to an UiEvent|Event when an image fails to load. Consumers should type check the value.

Steps to test

Run yarn run test
Verify tests pass
Run yarn run dev
Verify documentation

(optional) Implementation notes

At a high level, how did you implement this?

Upgraded Node.js
Upgraded dependencies to highest version we could support
Cleaned up test output throwing warnings and other errors
Pinned dependency resolutions as needed

Does this introduce any tech-debt items?

Yes #646

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical and brittle code paths are covered by unit tests
The change is described in the changelog section above

Reviewer guidance

Is the code clean and well-commented?
Are there tests for this change?
Are all UI components LTR and RTL compliant (if applicable)?
Add other things to check for here

After review

The changelog item has been pasted to the CHANGELOG.md

Comments

bjester · 2024-05-23T16:58:28Z

Before I actually update the changelog file, please let me know how my notes look in the PR.

… approach

bjester · 2024-05-23T18:48:45Z

Noting we must update the Node.js version in Netlify before merging

AlexVelezLl

Thank you @bjester! This looks great to me 👐, I left just some questions. And just a small note: Could you add your changelog items to CHANGELOG.md please?

lib/KImg/index.vue

lib/tabs/__tests__/KTabsList.spec.js

.github/workflows/tests.yml

AlexVelezLl · 2024-05-27T16:13:55Z

dev_docs/01_getting_started.md

- Node.js 10.x (see [Node Version Manager](https://github.com/nvm-sh/nvm))
- Yarn 1.x
+- Node.js 18.x (see [Node Version Manager](https://github.com/nvm-sh/nvm))
+- Yarn >=1.22.22


Now when I run yarn lint, yarn dev I get this error because I dont have pnpm installed:

> yarn lint yarn run v1.22.22 $ kolibri-tools lint --pattern '**/*.{js,vue,scss,less,css}' --ignore '**/node_modules/**,**/.nuxt/**,**/dist/**,**/lib/KIcon/precompiled-icons/**,**/lib/keen/**,**/docs/environment.js,**/docs/jsdocs.js' Kolibri CLI: Error: Command failed: "pnpm --version" stderr: Volta error: Could not find executable "pnpm" Use `volta install` to add a package to your toolchain (see `volta help install` for more info). Error details written to /home/alexvelezll/.volta/log/volta-error-2024-05-27_11_10_15.764.log original error message: Command failed: pnpm --version Volta error: Could not find executable "pnpm" Use `volta install` to add a package to your toolchain (see `volta help install` for more info). Error details written to /home/alexvelezll/.volta/log/volta-error-2024-05-27_11_10_15.764.log error Command failed with exit code 1.

Does the new version of kolibri-tools require pnpm to be installed? If so I think we need to add this to our prerequisites.

There should be no requirement on pnpm-- nothing in regards to that has changed.

Have you previously run a corepack command on your system?

I don't think this will resolve the issue, but you could try adding export SKIP_YARN_COREPACK_CHECK=1 to your terminal env.

Hmm, no, that didn't solve it. It's weird because I tried to run it on my Windows machine, where I also don't have Pnpm, and it worked there 🤔

In my Ubuntu machine I finally installed pnpm using volta install pnpm and then yarn lint worked.

bjester · 2024-05-28T17:50:49Z

Hmm linting was passing, but now it isn't 🤔 I'll take a look

MisRob · 2024-05-30T14:52:09Z

Before I actually update the changelog file, please let me know how my notes look in the PR.

Thank you, @bjester, to me it looks well and clear, and I appreciate the guidance

MisRob · 2024-06-05T09:47:39Z

Hi @bjester, thanks a lot - lots of work!

I've switched to this branch, then nvm use 18, then yarn install, and finally yarn dev. It seems the devserver initialization gets stuck and I can't access http://localhost:4000/ as usual

michaela@michaela:~/dev/kolibri-design-system$ yarn run dev
yarn run v1.22.17
$ npm-run-all --parallel dev-only _lint-watch-fix _api-watch pregenerate
$ yarn lint -w -m
$ chokidar "**/lib/**" -c "node utils/extractApi.js"
$ nuxt --port 4000
$ node utils/pregenerate.js
$ kolibri-tools lint --pattern '**/*.{js,vue,scss,less,css}' --ignore '**/node_modules/**,**/.nuxt/**,**/dist/**,**/lib/KIcon/precompiled-icons/**,**/lib/keen/**,**/docs/environment.js,**/docs/jsdocs.js' -w -m
ℹ  Netlify environment variables:                                     11:44:01
    NETLIFY:        undefined
    BRANCH:         undefined
    PULL_REQUEST:   undefined
    REPOSITORY_URL: undefined
ℹ Wrote environment to /home/michaela/dev/kolibri-design-system/docs/environment.js

 WARN  mode option is deprecated. You can safely remove it from nuxt.config

ℹ Wrote rst icon replacement strings to /home/michaela/dev/kolibri-design-system/docs/rstIconReplacements.txt

ℹ NuxtJS collects completely anonymous data about usage.          11:44:01 AM
  This will help us improve Nuxt developer experience over time.
  Read more on https://git.io/nuxt-telemetry

? Are you interested in participating? (Y/n) Kolibri Linter: Initializing watcher for the following patterns: **/*.{js,vue,scss,less,css}

 WARN  Could not extract docs from /home/michaela/dev/kolibri-design-system/lib/KThemePlugin.js

ℹ Wrote jsdocs to /home/michaela/dev/kolibri-design-system/docs/jsdocs.js
add:node_modules/.bin/acorn
add:node_modules/autoprefixer/node_modules/.bin/browserslist
add:node_modules/bonjour-service/node_modules/.bin/multicast-dns
...
add:node_modules/.bin/webpack-cli
add:node_modules/.bin/webpack-dev-server
Watching "**/lib/**" ..

 WARN  Could not extract docs from /home/michaela/dev/kolibri-design-system/lib/KThemePlugin.js

ℹ Wrote jsdocs to /home/michaela/dev/kolibri-design-system/docs/jsdocs.js

Can you see something problematic in the log above or have some recommendations on how to best proceed with debuggin?

MisRob · 2024-06-05T10:05:23Z

I wonder if this is related to the problem with the devserver.

yarn generate gives me this error

MisRob · 2024-06-05T10:06:32Z

Good news is that I could successfully run

yarn lint
yarn lint-fix
yarn lint-watch
yarn pregenerate
yarn dev-only
yarn precompile-svgs
yarn precompile-custom-svgs
yarn _lint-watch-fix
yarn test
yarn _api-watch

So it is only generate and dev causing trouble. I use Ubuntu.

MisRob · 2024-06-05T10:11:35Z

lib/KDateRange/KDateInput.vue

  import KTextBox from '../KTextbox';

+  let uuid = 0;


If we used the same approach from another component (instead of using uuid library), could there be any conflicts?

Only if the ID prefix wasn't already unique

MisRob · 2024-06-05T10:14:18Z

lib/KImg/index.vue

-          };
+          return defaultValue;
+        }
+        // Avoid accessing unset indices through `this.ratio`


MisRob

Also just read the code and haven't noticed any issues, just have one question about uuid

MisRob · 2024-06-05T16:10:45Z

@bjester Upgrading yarn to v1.22.22 as discussed helped me with yarn dev that works now. I still see problem with yarn generate. But we can have a look whether that's actually a regression or if it was present before (can try that out but later)

bjester · 2024-06-06T14:09:48Z

I have replicated @MisRob's issue. It seems to stem from incompatibility with hoisted postcss packages. I will look at fixing it

MisRob · 2024-06-19T03:42:56Z

I will unassign myself from the asigned reviewer role for now since everything is reviewed at this point. So that I don't block another round of review as I will be away for two weeks, back on July 8. If it's still open when I'm back and I can help, please ping me :)! Thanks.

bjester force-pushed the nodejs-upgrade branch from bbb275d to cea58c6 Compare May 23, 2024 14:05

bjester changed the title ~~Nodejs upgrade~~ Node.js upgrade to v18 along with dependency upgrades and linting fixes May 23, 2024

bjester marked this pull request as ready for review May 23, 2024 16:57

bjester requested review from MisRob and rtibbles May 23, 2024 16:57

bjester requested a review from AlexVelezLl May 23, 2024 17:57

bjester added 8 commits May 23, 2024 11:40

Update packages and configuration

14190d6

Fix failing tests post-upgrade, cleap up, replace UUID with alternate…

4300f04

… approach

Update dev docs and CI

3020dfd

Use https

0d82020

Pin resolution of postcss b/c 7<>8 incompatibilities; linting fixes

3ece1f1

Auto prettier/linting fixes

8a61249

Update imports to use node prefix

81d1c5b

Ensure events have doc strings

1e3f048

bjester force-pushed the nodejs-upgrade branch from b91addc to 1e3f048 Compare May 23, 2024 18:44

bjester changed the base branch from develop to release-v4 May 23, 2024 18:45

AlexVelezLl reviewed May 27, 2024

View reviewed changes

Update node version in publish workflow

96495ce

Linting fix

b80a56c

Update changelog

0beb457

MisRob reviewed Jun 5, 2024

View reviewed changes

lib/KImg/index.vue

};

return defaultValue;

}

// Avoid accessing unset indices through `this.ratio`

Copy link

Member

MisRob Jun 5, 2024

Choose a reason for hiding this comment

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

Nice

MisRob reviewed Jun 5, 2024

View reviewed changes

MisRob self-assigned this Jun 7, 2024

MisRob removed their assignment Jun 19, 2024

bjester mentioned this pull request Jun 25, 2024

Bump kolibri-tools, fix lint errors #672

Closed

AlexVelezLl mentioned this pull request Jul 3, 2024

Percy and jest-puppeteer environment setup for visual testing #670

Merged

9 tasks

rtibbles assigned radinamatic and unassigned radinamatic Jul 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Node.js upgrade to v18 along with dependency upgrades and linting fixes #645

Node.js upgrade to v18 along with dependency upgrades and linting fixes #645

bjester commented May 22, 2024 •

edited

Loading

bjester commented May 23, 2024

bjester commented May 23, 2024

AlexVelezLl left a comment

AlexVelezLl May 27, 2024

bjester May 28, 2024

bjester May 28, 2024

AlexVelezLl May 28, 2024 •

edited

Loading

bjester commented May 28, 2024

MisRob commented May 30, 2024

MisRob commented Jun 5, 2024

MisRob commented Jun 5, 2024

MisRob commented Jun 5, 2024

MisRob Jun 5, 2024

bjester Jun 6, 2024

MisRob Jun 5, 2024

MisRob left a comment

MisRob commented Jun 5, 2024 •

edited

Loading

bjester commented Jun 6, 2024

MisRob commented Jun 19, 2024

Node.js upgrade to v18 along with dependency upgrades and linting fixes #645

Are you sure you want to change the base?

Node.js upgrade to v18 along with dependency upgrades and linting fixes #645

Conversation

bjester commented May 22, 2024 • edited Loading

Description

Issue addressed

Changelog

Steps to test

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

Reviewer guidance

After review

Comments

bjester commented May 23, 2024

bjester commented May 23, 2024

AlexVelezLl left a comment

Choose a reason for hiding this comment

AlexVelezLl May 27, 2024

Choose a reason for hiding this comment

bjester May 28, 2024

Choose a reason for hiding this comment

bjester May 28, 2024

Choose a reason for hiding this comment

AlexVelezLl May 28, 2024 • edited Loading

Choose a reason for hiding this comment

bjester commented May 28, 2024

MisRob commented May 30, 2024

MisRob commented Jun 5, 2024

MisRob commented Jun 5, 2024

MisRob commented Jun 5, 2024

MisRob Jun 5, 2024

Choose a reason for hiding this comment

bjester Jun 6, 2024

Choose a reason for hiding this comment

MisRob Jun 5, 2024

Choose a reason for hiding this comment

MisRob left a comment

Choose a reason for hiding this comment

MisRob commented Jun 5, 2024 • edited Loading

bjester commented Jun 6, 2024

MisRob commented Jun 19, 2024

bjester commented May 22, 2024 •

edited

Loading

AlexVelezLl May 28, 2024 •

edited

Loading

MisRob commented Jun 5, 2024 •

edited

Loading