添加链接
link管理
链接快照平台
  • 输入网页链接,自动生成快照
  • 标签化管理网页链接

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

Describe the bug

SWC version v1.2.206 is causing our jest setup file to error out when calling jest.spyOn . v1.2.205 does not error out.

Input code

// jest.setup.ts
import * as cache from './packages/foo/src/common/cache'
const redis = new RedisMock(7481)
beforeEach(() => {
  jest.spyOn(cache, 'getRedis').mockResolvedValue(redis)
// cache.ts
import memoize from 'p-memoize'
import { getConfig } from './config'
export const getRedis = memoize(async () => {
  const config = await getConfig()
  return new Redis(config.redisUrl)

Config

"jsc": { "parser": { "syntax": "typescript", "decorators": true "target": "es2021", "keepClassNames": true

Playground link

No response

Expected behavior

jest.spyOn should stub cache.getRedis

Actual behavior

jest.spyOn fails with the following error:

TypeError: Cannot redefine property: getRedis
    at Function.defineProperty (<anonymous>)
    at ModuleMocker.spyOn (/home/circleci/project/node_modules/jest-mock/build/index.js:820:16)
    at Object.spyOn (/home/circleci/project/jest.setup.ts:26:8)
    at Promise.then.completed (/home/circleci/project/node_modules/jest-circus/build/utils.js:333:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (/home/circleci/project/node_modules/jest-circus/build/utils.js:259:10)
    at _callCircusHook (/home/circleci/project/node_modules/jest-circus/build/run.js:240:40)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at _runTest (/home/circleci/project/node_modules/jest-circus/build/run.js:202:5)

Version

v1.2.206+

Additional context

No response

@kdy1 thanks for looking into this. I'm trying to understand what the issue is and how to resolve it.

I believe #3843 is about stubbing an exported function that is called within its module. This is not what I'm doing here. Instead, I'm trying to stub an exported function that is called outside of its module, and it errors out on the jest.spyOn line, which I believe should be possible in commonjs. I do agree that it shouldn't be possible with ESM.

Our codebase is typescript transpiling to commonjs and we don't use ESM. This is why I don't understand your comment about aligning to ES specification, since this is a commonjs output.

Are you saying that SWC no longer supports stubbing functions exported by commonjs modules, as of v1.2.206, and should not going forward?

It's basically the same thing, and #3843 is not the only one about wrong usage of exports.
There were 7 (8 including this) issues and there was an issue for this case too

import * as cache from './packages/foo/src/common/cache'
jest.spyOn(cache, 'getRedis').mockResolvedValue(redis)

In ESM mode, you can never write anything on the exports from a module.
the spyOn method is trying to rewrite getRedis method which is forbid in esm mode.

@kdy1

If you want cjs behavior, you can use common js instead of ESM imports, or use import foo = require('foo') instead

Unfortunately, import foo = require('foo') will not fix this issue.
The key is how to export a module.
If the module is exported from an ESM module, you should not have a way to modify it directly from the outside.

In CJS world, you cannot import an ESM module, you will never run into this issue.

If I remember correctly, node can natively interoperate between ESM and CJS .
I will check the node interop behaviour.

If the module is exported from an ESM module, you should not have a way to modify it directly from the outside.

Oh, I see., thanks!

Just to clarify again: none of these modules (either jest.setup.ts or cache.ts) is ESM. They are typescript files that are transpiled to commonjs modules. I set them up so that they compile to commonjs, and I expect import * as cache from './cache' to transpile to a require, and to require a commonjs module.

Is the new behavior to transpile a module to ESM if it is required/imported using import? Would explicitly setting module.type to commonjs in swcrc fix this?

@ldiqual If you do not expect an esm behaviour, do not use esm syntax. use CJS module.exports, or CTS export synatx.

function getRedis() { }
export = {
    getRedis,
    foo,
          

@kdy1 I agree that the new behavior may not be a bug, but rather is fixing a previous bug, and the change results in a stricter / more correct adherence to the esm specification, but an argument can be made that this is a breaking change, and hence warrants a major version bump.

Furthermore, as @ldiqual said, I'm also using typescript, and our tsconfig.json has module set to CommonJS, module resolution to node, and target as es2020, so I'm not sure how esm is even coming into the picture.

Nope, I don't think breaking code depending on wrong behavior is a breaking change. It's a bugfix.

About ESM part, you used ESM syntax

Do I get CJS or ESM?
Both, and neither.

It is CJS, because the output code has exports/module.exports.
It is not CJS, because it is transformed from ESM.
Users are using ESM syntax, such as export default, export { foo }, etc.
Users expect ESM behaviour, such as export is live-binding, export cannot be overwritten.
These are ESM features. We simulate ESM behaviour in CJS.

If the user expects CJS, it is a misuse to use these syntaxes.
There is no equivalent export syntax in ESM.
However, the exports.foo = expr syntax still works, as will TS's export = expr.

But what if the user really wants ESM and wants to test in jest?
This is jest's problem, which is equivalent to how jest is used in an ESM environment.

If this is truly a bugfix and not a bug, you should update the intro text here: https://swc.rs/docs/usage/jest

This is no longer a drop in replacement for ts-jest as reverting to ts-jest reverts to the previous behavior

One workaround that I've found it so use ts-jest to transform just the module to be mocked (./packages/foo/src/common/cache in your case), and @swc/jest for the rest (in jest.config.js):

transform: {
  '^.+packages/foo/src/common/cache\\.ts$': 'ts-jest',
  '^.+\\.ts$': '@swc/jest'

It'll a bit slower, but if there aren't many tests requiring spying functionality, it might be acceptable.

Is this nuance/issue being left as it is or being worked on? I know there is some debate as to whether it should actually be ts-jest that are fixing their implementation but in the meantime it would be good for everyone looking to switch to SWC to have a solid workaround for this issue.

If it is not being worked on I think we need to remove the line about @swc/jest being a drop-in replacement for ts-jest as it simply isn't. The reasoning (who has the correct ESM implementation) is beside the point as just dropping in @swc/jest will 100% break a lot of working tests that are implementing spyOn in ts-jest and the current workaround seems to require that users place modules in separate files -- requires a lot of reworking for existing apps.

@kdy1 thanks for clarifying I just wanted to check so I knew as, philosophical debates aside, @swc/jest cannot be considered a drop-in replacement for ts-jest as there is a lot of reworking required in order to ensure currently passing tests continue to pass after swapping out the packages.

Other than putting pretty much every dependant function into its own module and then mocking it, do you have any other solutions/recommendations regarding this issue?

I think @swc/jest is still a drop-in replacement for ts-jest as it does not break the correct code.

See #5205