TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

tslib: "TypeError: Cannot redefine property:" Bug Appears To Have Returned

Open source-transformer opened this issue 3 years ago • 2 comments

Bug Report

Version 1.13.0 fixed the bug referenced here: https://github.com/microsoft/tslib/issues/102

But, in moving to either 2.0.0 or 2.1.0 - the bug is happening again.

🔎 Search Terms

tslib TypeError: Cannot redefine property:

🕗 Version & Regression Information

This is fixed in 1.13.0 - but broke again in both version 2.0.0 and 2.1.0

  • This prevents us from using tsconfig aliased paths in conjuncture with jest spyOn(..). We can work around this by providing the relative paths instead of the paths as determined by the tsconfig - but this is against our code policy.
  • This changed between versions 1.13.0 and 2.0.0

💻 Code

internal/business functions/directories scrubbed from the following code snippet - but in case this makes it more clear

import * as Utils from '@some-platform/core/libs/utils/support';

describe( 'test tslib compatibility with aliased imports', () => {

	it( 'jest spyOn', async () => {

		// test that we don't get the following error (tslib 2.0 and 2.1 will cause this):
		// TypeError: Cannot redefine property: foobar
		// at Function.defineProperty (<anonymous>)
		const foobarSpy = jest.spyOn( Utils, 'foobar' );

		const imported = await Utils.foobar( );

		expect( foobarSpy ).toHaveBeenCalled();
	} );
} );

🙁 Actual behavior

We're forced to use relative paths when we use jest spyOn type functionality - instead of the aliased paths from tsconfig.

🙂 Expected behavior

We shouldn't have to update our paths to be relative paths.

source-transformer avatar Mar 04 '21 20:03 source-transformer

I'm having the same issue when mocking node's path library return value:

import * as path from 'path';

describe('test path', () => {
  it('should mock path.resolve', () => {
    const pathSpy = jest.spyOn(path, 'resolve').mockReturnValue('/');
    path.resolve('my/unexisting/path');
    expect(pathSpy).toHaveBeenCalledWith('my/unexisting/path');
  });
});

Will throw: TypeError: Cannot redefine property: resolve

Changing the way that Typescript imports the module:

import path from 'path'

instead of

import * as path from 'path'

With this change the test passes but a jest error is thrown, not related to this issue:

Test suite failed to run

    Cannot find module 'jest-util'
    Require stack:
    - /test-project/node_modules/jest-runtime/build/index.js
    - /test-project/node_modules/@jest/core/build/cli/index.js
    - /test-project/node_modules/@jest/core/build/jest.js
    - /test-project/node_modules/jest-cli/build/cli/index.js
    - /test-project/node_modules/jest-cli/bin/jest.js
    - /test-project/node_modules/jest/bin/jest.js

      at _jestUtil (node_modules/jest-runtime/build/index.js:164:16)

So consider importing just what you need (named imports) instead of importing all (*) from a module.

victorigualada avatar Mar 24 '21 13:03 victorigualada

Hi, Any updates on the fix for this bug ? This is preventing us from upgrading tslib and typescript as we have 1000+ tests which would need to have a workaround with jest

utsavkapoor avatar Oct 18 '22 02:10 utsavkapoor

This is especially problematic because I find jest.mock() to be a black box of buggy behavior.

ericbiewener avatar Oct 31 '22 21:10 ericbiewener

I got this everywhere in my code and look forward to when I can delete it

// delete when resolved: https://github.com/microsoft/TypeScript/issues/43081
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
jest.mock('foo', () => ({
  __esModule: true,
  ...jest.requireActual('foo'),
}));

jethrolarson avatar Dec 14 '22 23:12 jethrolarson

It looks like the issue is that the __createBinding helper needs to have configurable: true so that Jest can overwrite the property. @weswigham do you see any reason why making these properties configurable would be a problem?

rbuckton avatar Mar 14 '24 22:03 rbuckton

Following discussion in the Design Meeting, we've decided not to take a fix for this as we would be inconsistent with esbuild, babel, et al. There are already workarounds for this, such as using jest.unstable_moduleMock, and this seems like an issue Jest would need to address in their mock implementation so as to be compatible with other transpilers, such as replacing the module object, introducing a Proxy, etc.

rbuckton avatar Apr 02 '24 21:04 rbuckton