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

Upgrade to Chai 5.x #10732

Merged
merged 4 commits into from
Dec 11, 2024
Merged

Upgrade to Chai 5.x #10732

merged 4 commits into from
Dec 11, 2024

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented Dec 7, 2024

Closes #10720
Closes #10721

There are 4 key changes here:

1. Remove chai-string

Chai-string is not compatible with chai 5
I did consider submitting a PR upstream, but the last commit to https://github.com/onechiporenko/chai-string was 5 years ago, it has no working CI build and the most recent node version the package was tested against is node 10. As such, I think there is quite a high risk this package is abandoned and my PR will be ignored.

Given we only use chai-string assertions in a handful of places, I have just opted to rewrite those few tests to use native chai and javascript so we can remove the plugin completely.

2. Upgrade chai and sinon-chai

These 2 packages need upgrading together to ensure due to peer dependency constraints.

3. Update chai imports

One of the breaking changes in Chai 5.0.0 (see https://github.com/chaijs/chai/releases/tag/v5.0.0 ) is

Chai now only supports EcmaScript Modules (ESM). This means your tests will need to either have import {...} from 'chai' or import('chai')

This means we needed to update any places where we are just doing import chai from 'chai'

4. Migrate badge-maker tests to ESM

Most of the shields codebase is written in ESM. The badge-maker package is still CommonJS. We might at some point want to migrate the package to ESM anyway but I don't think we should be forced into that by a testing package. Lets consider that decision on its own merits. I have kept the package as CJS for now but migrated just the 3 test modules that use Chai to ESM by using the .mjs extension.

@chris48s chris48s added developer-experience Dev tooling, test framework, and CI dependencies Related to dependency updates labels Dec 7, 2024
Copy link
Contributor

github-actions bot commented Dec 7, 2024

Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against bf040eb

Copy link

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/chai@5.1.2 None +3 549 kB chaijs

🚮 Removed packages: npm/chai-string@1.5.0, npm/chai@4.5.0

View full report↗︎

@chris48s chris48s changed the title WIP Chai upgrade Upgrade to Chai 5.x Dec 7, 2024
@chris48s chris48s marked this pull request as ready for review December 7, 2024 14:52
@chris48s chris48s requested review from PaulaBarszcz and jNullj and removed request for PaulaBarszcz December 7, 2024 14:54
Copy link
Member

@PyvesB PyvesB left a comment

Choose a reason for hiding this comment

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

Looks good to me! 👍🏻

@chris48s chris48s removed the request for review from jNullj December 11, 2024 19:28
@chris48s chris48s added this pull request to the merge queue Dec 11, 2024
Merged via the queue into badges:master with commit fb816ec Dec 11, 2024
32 checks passed
@chris48s chris48s deleted the chai-upgrade branch December 11, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Related to dependency updates developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants