dom-testing-library
dom-testing-library copied to clipboard
chore(deps): replace chalk with picocolors
Replaced chalk with picocolors as discussed in #1340.
I ended up using picocolors to support the NO_COLOR env (https://no-color.org/)
Checklist:
- [ ] Documentation added to the docs site N/A
- [ ] Tests
- [ ] TypeScript definitions updated N/A
- [ ] Ready to be merged
I wasnt able to run the tests. It errored with TypeError: _browserslist.findConfigFile is not a function.
I dont want to call this mergable unless I can see the green tests.
// EDIT: The CI seems to fail with the same error
// OFFTOPIC: I saw that @testing-library/jest-dom is also using chalk but only in a test. However, it is listed as dependency. Maybe we can move it to devDependencies and also replace it?
I saw that @testing-library/jest-dom is also using chalk but only in a test. However, it is listed as dependency. Maybe we can move it to devDependencies and also replace it?
It's listed in devDependencies as far as I can tell.
CI errors may be happening without this change and need an update to our build pipeline. Do you think you can fix it?
@eps1lon I looked into it and this is a deadlock. I tried several combinations of pinned dependencies and got the tests to work eventually. But now validate fails.
I really wonder why nobody ever bothered to commit a lock-file. Thats exactly what it is for :D.
The reason this is all not working is, is because some babel dependencies that are brought in require at least babel 7.22. However, that version calls browserlist with a different api (and therefore fails). Installing older versions of those packages gets rid of that error but now the validate fails with (to me unknown) reason because it complains that a function isnt found (which is clearly there).
So I have to give up for now. What other possibilities do we have? Does anyway of you have a working setup? If so, please dump all the exact versions of your dependencies by (deleting any shrinkwrap and lock file and then) running
npm shrinkwrap
Give me that file :D
// EDIT: CI: it looks like its a dependency and not in dev: https://github.com/testing-library/jest-dom/blob/main/package.json
This pull request is automatically built and testable in CodeSandbox.
To see build info of the built libraries, click here or the icon next to each commit SHA.
Latest deployment of this branch, based on commit 1bb91ead71ccf539d5081eb88ba68a7dc5f048a9:
| Sandbox | Source |
|---|---|
| react-testing-library-examples | Configuration |
OMG I DID IT!!!! I had to commit the package-lock.json to convince the CI to install the correct versions. So not sure what to do with this.
It would be really beneficial to get rid of old supported browsers so we can update browserslist so we can update babel so we can get rid of all that overwrites and really unstable depenceny situation
@timdeschryver I reverted the commit of the package-lock and the ci is red again. So would love some help here. Clearly the CI does something with the lock-file :D
Also moved typescript back to devDependencies where it rightfully belongs
It seems like that there was a push to introduce a lockfile in 2020 already but Kent wanted the approval from one other maintainer (https://github.com/testing-library/dom-testing-library/pull/624#issuecomment-643260300). Maybe that could be you? :D
Kent also seems to use lockfiles now: https://github.com/kentcdodds/kentcdodds.com
Kent also seems to use lockfiles now: https://github.com/kentcdodds/kentcdodds.com
@Fuzzyma Kent's not opposed to using lock files in projects/apps, but the way I understood it he is opposed using them in libraries/packages
I will take a look at this in the next days (I might push directly into this branch). AFAIK @MichaelDeBoey is right about the reasons of why not to include a lock file, additionally it also allows people to use their package manager of choice. We might revisit this later, but I prefer not to do this within this PR.
@eps1lon does this seem good to you?
As mentioned in the comments, @babel/helper-compilation-targets was pinned to make the build happy without upgrading deps/changing the browserlist support (as was done in #1346)
This has three approvals. Could we merge it?
I would also recommend merging this related PR: https://github.com/testing-library/jest-dom/pull/659 (chalk 3 is used there, so we're currently pulling in two versions of chalk. I.e. one from here and one from there)
Thanks @benmccann for the ping. I've just merged https://github.com/testing-library/jest-dom/pull/659, let's see we're not seeing any regressions there and I'll merge this one in the upcoming week :)
Thank you!!
Thank you!