feat: decouple from cross-fetch

[dirkluijk]

All right, this PR includes:

• Moving away from cross-fetch by just patching the global fetch which should be present in all modern runtimes and browsers. That would close issue https://github.com/IanVS/vitest-fetch-mock/issues/22 and https://github.com/IanVS/vitest-fetch-mock/issues/23.
• It will now force Node 18, which closes issue https://github.com/IanVS/vitest-fetch-mock/issues/21 and removes the need to patch the DOMException.
• I added a new feature that automatically registers enables in beforeAll, resets in beforeEach and disables in afterAll (not yet documented). this feature was dropped
• The source code is now written in TypeScript and transpiled to JavaScript into the dist directory, including the type definitions and source maps.
• The package is now published in ESM format. That could be considered a breaking change. I personally believe everyone should push to modern web standards and use ESM, but that is only my personal opinion. I could revert this, allthough Vitest should be able to handle ESM (actually, it embraces ESM). Never mind, it was already being published in ESM format before.

[dirkluijk]

@IanVS I changed the workflow a bit, so you will have to approve it before it runs :)

[dirkluijk]

Pushes a new commit, changes:

• merged the interface and the object into a single class definition, did some rework on the code that merges the Mock and the additional methods into the FetchMock
• removed the autoEnable features
• restored some aliases that I forgot to add back in (my first plan was to rewrite the whole thing, removing a lot of aliases, but then decided that wasn't a good idea for an existing library...)
• reverted some changes in the tests
• fixed inconsistencies between overloads and the actual original implementation, by making the latter leading and making the types a little bit more flexible
• made createFetchMock the default export again
• adjusted the NPM scripts
• adjusted some methods to match the original signatures

[dirkluijk]

I notice one final change here that I want to ask about. Previously it was up to the user to provide vi to the createFetchMock function. Now, you're importing it straight from vitest. This makes me slightly nervous because it means we are relying on both this package and the user's code pulling the same version of vitest. We have a peer dependency on vitest, so theoretically it should work fine, but I've struggled more than I like to recall with different versions of packages being used by different dependencies causing very subtle and hard-to-troubleshoot bugs.

Can you talk a little about what you see as the pros & cons of this new approach here vs the explicit dependency injection we currently have? And furthermore, are you maybe willing to move that change to a separate PR (can group together with the auto-enabling if you'd like), so that we can keep this particular PR focused on decoupling from cross-fetch?

The signature change was one of the many changes I initially made. At that point it was more of a simple proof-of-concept, written from scratch, using this library as inspiration. When I started adding more and more back in, I started to consider it as some kind of fork (but still, with many different design decisions). Only later I decided to contribute this as a pull request to this project, and with that approach we have all the reason to be careful. I see no reason to break this interface now. That’s why reverted the change. 😉

Is there a difference? Well, that depends.

• When this library would use Vitest only as dependency, and creates the mocks by importing from Vitest as its own dependency, you could in theory allow users to have their own version of Vitest, even if it's a conflicting version. In case of conflicts, NPM will put "our" version of vitest in the directory node_modules/vitest-fetch-mock/node_modules/vitest and the users version of vitest would be located in node_modules/vitest. That is perfectly fine, as long as it's not part of any public interface (that is, as return value or as argument). But when you break this rule, you could get weird errors since the argument or return value is possibly not assignable to the users type-definition. Remember, it's usually TypeScript complaining here, since at runtime it might possibly still work fine. I believe this is the situation you are referring to. It's basically caused by library publishers that made a mistake here.
• However, in our situation Vitest is part of our public interface, since we extend Mock from @vitest/spy in our return types. In that case you must always specify it as devDependency and peerDependency. This should be 100% safe, since for the user, there is really only one package of Vitest being installed (in node_modules/vitest). There are two caveats: first, when the user doesn't have skipLibCheck enabled, I believe there might be some cases where you get type-check errors in case of inconsistencies, but they are rare and the best practice is to always disable lib checks. Secondly, if the installed version has some breaking change somewhere internally, it could still lead to unintended behavior. But this should not happen, and will probably not happen with a library as popular as Vitest.

With the peer dependency approach, there is not really any value over providing the vi instance as argument compared to importing it ourselves. The result is exactly the same - both in runtime behavior and in compiletime type-checking. That's why I removed it initially, but reverting is better to not break the interface. Alternatively, we could also mark it as optional.

[dirkluijk]

Added another small fix that prevented types/test.ts from type-checking before. That led me to making isMocking public again, and adjusting the ErrorOrFunction type-definition a bit.

I can't find any remaining issues anymore. I'm quite confident about this PR.

[IanVS]

whoops, spoke too soon, looks like there's a minor linting issue.

[dirkluijk]

fixed!

[IanVS]

Thanks for contributing this as a PR even though I'm sure it was much more annoying to do than just making a fork or new project.

If you'd like to make more breaking change PRs, for things like auto-enable and relying on peer dependencies, I'd be happy to review/merge those as well before making a 0.4 release (or maybe we just go to 1.0.0 now).

I think I'm on board with the peer dependency, though there are some edge cases that could still bite us, like npm automatically installing a peer dependency when it doesn't find one, and yarn doing weird things in monorepos sometimes. But I think in our case it's probably pretty low risk.

[dirkluijk]

Thanks for contributing this as a PR even though I'm sure it was much more annoying to do than just making a fork or new project.

No worries, it actually reminds me to care about breaking changes, which is absolutely a good thing!

If you'd like to make more breaking change PRs, for things like auto-enable and relying on peer dependencies, I'd be happy to review/merge those as well before making a 0.4 release (or maybe we just go to 1.0.0 now).

My feeling right now is to release this as 0.4, then add new feature(s) in 0.5, and then keep 1.0 for when things are stable and maybe for introducing some breaking changes and/or mark some things as deprecated to clean things up (and remove them for real in 2.0).

If you agree, I'd say, let's release and publish the new version. 🎉

I think I'm on board with the peer dependency, though there are some edge cases that could still bite us, like npm automatically installing a peer dependency when it doesn't find one, and yarn doing weird things in monorepos sometimes. But I think in our case it's probably pretty low risk.

As far as I can see, nothing changed there. Vitest was already a peer dependency before 😉