Skip to content

Node.js 18, pnpm 8, jest 29, linting/formatting updates, and webpack fixes #4462

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

Merged
merged 11 commits into from
Apr 29, 2025

Conversation

bjester
Copy link
Member

@bjester bjester commented Feb 29, 2024

Summary

Description of the change(s) you made

  • Upgrades documentation, docker images, and any other references to use Node.js 18
  • Upgrades several JS dependencies to newer versions:
    • jest & co to v29
    • KDS to v5
    • fake-indexeddb to v5
    • kolibri-tools and kolibri-format to latest
    • autoprefixer to v10
    • stylus-loader to v8
  • Fixes issues with tests after Jest upgrade
  • Ran pnpm run lint-frontend:format

Manual verification steps performed

  1. Ran pnpm run test-jest
  2. Ran pnpm run devserver:hot

References

Requires: learningequality/kolibri#12052

Comments

@rtibbles rtibbles self-assigned this Mar 12, 2024
@bjester bjester changed the title Nodejs 18, Jest 29, and linting/formatting updates Node.js 18, pnpm 8, jest 29, linting/formatting updates, and webpack fixes Apr 5, 2024
@bjester
Copy link
Member Author

bjester commented Jun 19, 2024

Need to check whether we can upgrade less to 4.x

@bjester bjester marked this pull request as ready for review February 25, 2025 16:23
@bjester bjester force-pushed the nodejs-18 branch 7 times, most recently from cf8f431 to d4b7f6c Compare February 25, 2025 19:22
@bjester bjester closed this Feb 25, 2025
@bjester bjester reopened this Feb 25, 2025
@bjester bjester force-pushed the nodejs-18 branch 2 times, most recently from 7edf4e8 to c52f2a9 Compare April 4, 2025 16:03
@bjester bjester force-pushed the nodejs-18 branch 3 times, most recently from 509d739 to 58a0221 Compare April 7, 2025 21:11
rtibbles
rtibbles previously approved these changes Apr 9, 2025
Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

I have reviewed all the commits that it would be safe to do so/that I haven't already reviewed. One possible issue in the flake.nix file, but it already seems to be non-functional, so I am not sure we actually need to block this PR for it.

build_assets:
name: Build frontend assets
needs: pre_job
if: ${{ needs.pre_job.outputs.should_skip != 'true' }}
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Use pnpm
Copy link
Member

Choose a reason for hiding this comment

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

I was a little surprised that this comes before the Node setup, but not so surprised that I care to understand why it is suggested to be this way in the documentation!

@rtibbles rtibbles dismissed their stale review April 9, 2025 18:54

I got a bit over eager - going to manually test the pnpm workflow first!

Copy link
Member

@rtibbles rtibbles left a comment

Choose a reason for hiding this comment

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

Manual testing of the switchover to pnpm (using volta) shows no issues, absolutely seamless.

@bjester bjester merged commit c911e14 into learningequality:unstable Apr 29, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants