Skip to content
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

Convert to ESM #78

Open
4 tasks
CMCDragonkai opened this issue Dec 16, 2024 · 4 comments
Open
4 tasks

Convert to ESM #78

CMCDragonkai opened this issue Dec 16, 2024 · 4 comments
Labels
procedure Action that must be executed

Comments

@CMCDragonkai
Copy link
Member

This package should be possible to convert to ESM, all its dependencies are already ESM.

Tasks

  • 1. Update all dependencies to ESM
  • 2. Migrate tests
  • 3. Migrate package.json
  • 4. Migrate src code
@CMCDragonkai CMCDragonkai added the procedure Action that must be executed label Dec 16, 2024
Copy link

linear bot commented Dec 16, 2024

ENG-504 Convert to ESM

@CMCDragonkai
Copy link
Member Author

Ok migrating to ESM will hit a problem with the way fastcheck is currently used.

See: dubzzz/fast-check#4986 (comment)

It's when fastcheck is integrated with jest that's when there's a problem.

@amydevs had done something before to as a hack here.

It has something to do with:

  • fastcheck
  • jest
  • swc

@CMCDragonkai
Copy link
Member Author

To make this work, we have a few options:

  1. not use the fast check jest integration, and just use fast check directly like we used to - a bit annoying, but not really that bad
  2. Always monkey patch fast check jest integration to remove its import of @jest/globals as per @amydevs comment - this would only solve it in the case where fast check jest is doing this (its the only dependency where this is occurring)
  3. Modify the transform chain as per https://github.com/swc-project/jest/issues/120#issuecomment-1770662124 somehow so that this prevents any problem with any new test dependency that might do that importing

The core of the problem is that jest mocking is not hoisted above ESM module imports, unlike CJS imports, and moving to ESM means that mocking won't really work.

@CMCDragonkai
Copy link
Member Author

To make this work, we have a few options:

  1. not use the fast check jest integration, and just use fast check directly like we used to - a bit annoying, but not really that bad
  2. Always monkey patch fast check jest integration to remove its import of @jest/globals as per @amydevs comment - this would only solve it in the case where fast check jest is doing this (its the only dependency where this is occurring)
  3. Modify the transform chain as per Add hoisting of the "@jest/globals" import together with jest.mock swc-project/jest#120 (comment) somehow so that this prevents any problem with any new test dependency that might do that importing

The core of the problem is that jest mocking is not hoisted above ESM module imports, unlike CJS imports, and moving to ESM means that mocking won't really work.

I think solution 1. could be achieved by writing our own fast check jest integration. It's a pretty simple package: https://github.com/dubzzz/fast-check/tree/main/packages/jest. It would be a simple utility package, rather than something large, so we wouldn't bother with even testing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
procedure Action that must be executed
Development

No branches or pull requests

1 participant