Update to chai@5 instead of @esm-bundle/chai #2795
Conversation
🦋 Changeset detectedLatest commit: 1b468ab The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
| @@ -9,7 +8,13 @@ const { startDevServer } = require('@web/dev-server'); | |||
|
|
|||
| const rootDir = path.resolve(__dirname, '..', 'dist'); | |||
|
|
|||
| describe('integration tests', () => { | |||
| describe('integration tests', async () => { | |||
| let expect; | |||
There was a problem hiding this comment.
cant we just
const { expect } = await import('chai');instead of doing it in a before block?
There was a problem hiding this comment.
I was doing that before but had to change to this because I was getting silent failures in mocha. Some googling around brought me to mochajs/mocha#2975 (comment) which seemed to indicate that wrapping it in a before would fix the issue. And it seems to fix it for me.
There was a problem hiding this comment.
It's a bit weird. This can then all be removed once this is all ESM.
| describe('integration tests', async () => { | ||
| let expect; | ||
| before(async () => { | ||
| const chai = await import('chai'); |
There was a problem hiding this comment.
({expect}=async import(‘chai’));?
There was a problem hiding this comment.
Happy to do this if you have a strong opinion but if it's ok I'll leave this for now. There's once place at least where we need to call chai.use(..) in between anyways and I think we can stop doing this in a before once we are using ESM anyways.
There was a problem hiding this comment.
Separately, does @open-wc/testing do this for consumers, already? Or will this be a requirement to migrating?
There was a problem hiding this comment.
Not a blocker. Mainly pointing it out in the case end users need to do this every time.
There was a problem hiding this comment.
Yeah we do this for consumers. We load up all the plugins and then return a instance of chai with those loaded. Found a issue with chai while checking that out that we gotta fix before landing this: https://github.com/open-wc/open-wc/pull/2795/files#r1599496849
| import { chaiDomDiff } from '@open-wc/semantic-dom-diff'; | ||
|
|
||
| window.chai.use(chaiDomDiff); | ||
| chai.use(chaiDomDiff); |
There was a problem hiding this comment.
This needs to be const chai = use(chaiDomDiff); actually.
| @@ -2,7 +2,7 @@ | |||
| import { chaiDomDiff } from '@open-wc/semantic-dom-diff'; | |||
| import { chaiA11yAxe } from 'chai-a11y-axe'; | |||
|
|
|||
| import chai from '@esm-bundle/chai'; | |||
| import * as chai from 'chai'; | |||
There was a problem hiding this comment.
Running into chaijs/chai#1603 when trying to register all the plugins further down in this file. It needs to be fixed before we can land this PR.
I would like to update to chai@5 as a replacement for
@esm-bundle/chai. I additionally needed to upgradesinon-chai.sinon-chaiis currently in a prerelease phase so I updated to that one. If this canary release goes well we can make a PR tomasterand communicate that the prerelease ofsinon-chaiis working as expected to thechaimaintainers.