-
-
Notifications
You must be signed in to change notification settings - Fork 186
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
Better interrupt CJS-ESM as we provide both in our bundle (Symbol.for switch) #4845
Labels
Comments
This code should pass: import { expect, test } from "vitest";
import fc from "fast-check";
const fcRequire = require("fast-check");
test("should pass as pre protect ourselves", () => {
fc.assert(
fc.property(fc.nat(), (n) => {
fcRequire.pre(n % 2 === 0);
expect(n % 2).toBe(0);
})
);
}); |
dubzzz
added a commit
that referenced
this issue
May 14, 2024
fast-check was badly behaving on cloning method, custom toString and assertions linked to pre when several versions of it were running at the same time. One of the case for that issue is ES module and CommonJS being mixed in the same test and leading fast-check to be loaded once in ESM and once in CJS. Another case is that user relies on two versions of fast-check due to a dependency forcing a version that is not the same as the one requested by the user or by another dependency (not the same meaning not compatible semver ranges). Fixes #4845
12 tasks
dubzzz
added a commit
that referenced
this issue
May 14, 2024
fast-check was badly behaving on cloning method, custom toString and assertions linked to pre when several versions of it were running at the same time. One of the case for that issue is ES module and CommonJS being mixed in the same test and leading fast-check to be loaded once in ESM and once in CJS. Another case is that user relies on two versions of fast-check due to a dependency forcing a version that is not the same as the one requested by the user or by another dependency (not the same meaning not compatible semver ranges). Fixes #4845 <!-- Context of the PR: short description and potentially linked issues --> <!-- ...a few words to describe the content of this PR... --> <!-- ... --> <!-- Type of PR: [ ] unchecked / [ ] checked --> **_Category:_** - [ ] ✨ Introduce new features - [ ] 📝 Add or update documentation - [ ] ✅ Add or update tests - [ ] 🐛 Fix a bug - [ ] 🏷️ Add or update types - [ ] ⚡️ Improve performance - [ ] _Other(s):_ ... <!-- Don't forget to add the gitmoji icon in the name of the PR --> <!-- See: https://gitmoji.dev/ --> <!-- Fixing bugs, adding features... may impact existing ones --> <!-- in order to track potential issues that could be related to your PR --> <!-- please check the impacts and describe more precisely what to expect --> **_Potential impacts:_** <!-- Generated values: Can your change impact any of the existing generators in terms of generated values, if so which ones? when? --> <!-- Shrink values: Can your change impact any of the existing generators in terms of shrink values, if so which ones? when? --> <!-- Performance: Can it require some typings changes on user side? Please give more details --> <!-- Typings: Is there a potential performance impact? In which cases? --> - [ ] Generated values - [ ] Shrink values - [ ] Performance - [ ] Typings - [ ] _Other(s):_ ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
🐛 Bug Report
Related to #4842
Until we fully drop dual package, we should probably switch back to
Symbol.for
to drop risky things linked to symbols not being the same in different realms.The text was updated successfully, but these errors were encountered: