-
-
Notifications
You must be signed in to change notification settings - Fork 771
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
Add explicit export for ./*
#2413
Conversation
This is great, so thanks for this. But to avoid issues of this kind I would like to not rely on after-the-fact bug reports, but have our own little set of integration tests of sorts to catch these things when they happen. So could you instruct me of what a minimal setup for reproduction of this issue would be? Something a la
Having that, we could run this set of integration tests before every release. |
Let me try come with a small test that can repro the issue. |
Did a little bit more digging and the issue is caused by this line in karma-sinon-chai: |
Opened PR against karma-sinon-chai: kevicency/karma-sinon-chai#64 If it get fixed in karma-sinon-chai, I think this PR might not be necessary. Let's see. |
Hey dude, in case you are interested, I setup some |
@perrin4869 That was a 404 when I checked. |
Sorry, in the meantime it changed to |
Since kmees/karma-sinon-chai seems quite dead, I assume that fix is not landing. I had quick glance the karma config and the karma-runner/karma-mocha, but I could not immediately figure out how to create a headless test (though I saw the |
package.json
./*
Chai.js has recently merged a change that fixes this issue by adding export for |
This feels a bit scary to merge without any tests of typical consumers ... I just remember that every time we have touched the We could of course just ship a patch version and see what noise comes up. If it's breaking in some way, we could revert or fix it. |
The rationale behind this:
If you like we can wait a week or two and see if any issue arise on chai.js regarding the change on their side. |
Thanks for the explanation! Will release a new major version anyway after upgrading the fake-timers package. |
Purpose
Add an explicit export entry for
package.json
to fix the following error when running karma:Background
nodejs/node#33460
There is a lot of discussion around whether declaring
package.json
as "public API" viaexports
is a good or bad idea. My take is simply that we need to expose this as a workaround before node team decide what to do, otherwise it breaks many tools that relies onpackage.json
.How to verify
Dependabot update sinon 12.0.0 failed on CI and manually patching
package.json
tested and working locally.E.g. https://github.com/ntkme/github-buttons/runs/4099992660?check_suite_focus=true
Checklist for author
npm run lint
passes