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

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

Closed
ghost opened this issue Mar 4, 2021 · 6 comments
Closed
Assignees
Labels
Rescheduled This issue was previously scheduled to an earlier milestone Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@ghost
Copy link

ghost commented Mar 4, 2021

Bug Report

Version 1.13.0 fixed the bug referenced here:
microsoft/tslib#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.

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Mar 4, 2021
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.3.0 milestone Mar 4, 2021
@victorigualada
Copy link

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.

@utsavkapoor
Copy link

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

@ericbiewener
Copy link

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

@jethrolarson
Copy link

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'),
}));

@rbuckton
Copy link
Member

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
Copy link
Member

rbuckton commented Apr 2, 2024

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 rbuckton closed this as not planned Won't fix, can't repro, duplicate, stale Apr 2, 2024
@rbuckton rbuckton added Working as Intended The behavior described is the intended behavior; this is not a bug and removed Fix Available A PR has been opened for this issue labels Apr 2, 2024
@rbuckton rbuckton removed the Needs Investigation This issue needs a team member to investigate its status. label Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Rescheduled This issue was previously scheduled to an earlier milestone Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
8 participants