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

A TypeError occurred when mocks BatchResponseContent via jest.spyOn() #514

Closed
5 of 6 tasks
karamem0 opened this issue Sep 14, 2021 · 9 comments · Fixed by #527
Closed
5 of 6 tasks

A TypeError occurred when mocks BatchResponseContent via jest.spyOn() #514

karamem0 opened this issue Sep 14, 2021 · 9 comments · Fixed by #527
Assignees

Comments

@karamem0
Copy link

Bug Report

Prerequisites

  • Can you reproduce the problem?
  • Are you running the latest version?
  • Are you reporting to the correct repository?
  • Did you perform a cursory search?

For more information, see the CONTRIBUTING guide.

Description

Test failed when I upgraded SDK version from 2.2.1 to 3.0.0.

Console Errors: TypeError: Cannot redefine property: BatchResponseContent

Screenshots:
Screenshots

Steps to Reproduce

  1. Create a new project. npm init
  2. Install packages.
  • @microsoft/microsoft-graph-client@3.0.0
  • @types/jest@26.0.24
  • jest@26.6.0
  • ts-jest@26.5.6
  • typescript@4.3.5
  1. Write a test script. index.test.ts
import * as microsoftGraph from '@microsoft/microsoft-graph-client';
it('test', () => {
    jest.spyOn(microsoftGraph, 'BatchResponseContent');
});

Expected behavior: Test passed.

Actual behavior: Test failed because cannot mock BatchResponseContent.

Additional Context

It works with @microsoft/microsoft-graph-client@2.2.1. Was it any changes?

Usage Information

Request ID - Value of the requestId field if you are receiving a Graph API error response

SDK Version - 3.0.0

  • Node (Check, if using Node version of SDK)

Node Version - 14.17.6

  • Browser (Check, if using Browser version of SDK)

Browser Name - [The name of Browser that you are using for SDK]

Version - [The version of the browser you are using]

@nikithauc
Copy link
Contributor

@karamem0 There are no changes to BatchResponseContent in 3.0.0.

@karamem0
Copy link
Author

@nikithauc Thank you for replying. Did you try to reproduce the issue? In actual there might be no changes to BatchResponseContent but something causes the error.

@nikithauc
Copy link
Contributor

@karamem0 I was able to reproduce the issue. Will have to look more into this to understand the cause of the issue.

Thank you for reporting this!

Are you able to use the BatchResponseContent otherwise?

@nikithauc
Copy link
Contributor

nikithauc commented Sep 24, 2021

@karamem0
I came across some similar reported issues:

aelbore/esbuild-jest#26
https://stackoverflow.com/questions/67872622/jest-spyon-not-working-on-index-file-cannot-redefine-property
https://medium.com/walmartglobaltech/jest-v18-to-v23-migration-notes-b726cf6aafcf

The typescript version was updated to version 4.x in @microsoft/microsoft-graph-client@3.0.0. My first thought is this upgrade is causing the issue.

Does the workaround suggested as follows work for you?
https://stackoverflow.com/questions/67872622/jest-spyon-not-working-on-index-file-cannot-redefine-property

@ghost
Copy link

ghost commented Sep 29, 2021

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.

@karamem0
Copy link
Author

karamem0 commented Oct 2, 2021

@nikithauc
Thanks a lot! I understood the reason is the change of typescript build systems.
I will use jest.mock instread of jest.spyOn.

jest.mock('@microsoft/microsoft-graph-client');
import { BatchResponseContent } from '@microsoft/microsoft-graph-client';
it('test', () => {
  const MockBatchResponseContent = BatchResponseContent as unknown as jest.Mock;
  MockBatchResponseContent.mockImplementation(() => ({
    getResponses: jest.fn()
  }));
});

@karamem0
Copy link
Author

karamem0 commented Oct 4, 2021

My suggestion to avoid this error is as follows.

before:

export * from "./content/BatchRequestContent";

after:

export { BatchRequestContent } from "./content/BatchRequestContent";

@nikithauc
Copy link
Contributor

@karamem0 Thanks for bringing this up!

Can you explain the difference here and how that caused the error?

My suggestion to avoid this error is as follows.

before:

export * from "./content/BatchRequestContent";

after:

export { BatchRequestContent } from "./content/BatchRequestContent";

@karamem0
Copy link
Author

karamem0 commented Oct 8, 2021

@nikithauc

If build using export *, the result is like below.

"use strict";
var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) {
    if (k2 === undefined) k2 = k;
    Object.defineProperty(o, k2, { enumerable: true, get: function() { return m[k]; } });
}) : (function(o, m, k, k2) {
    if (k2 === undefined) k2 = k;
    o[k2] = m[k];
}));
var __exportStar = (this && this.__exportStar) || function(m, exports) {
    for (var p in m) if (p !== "default" && !Object.prototype.hasOwnProperty.call(exports, p)) __createBinding(exports, m, p);
};
Object.defineProperty(exports, "__esModule", { value: true });
__exportStar(require("./content/BatchRequestContent"), exports);

__exportStar function calls Object.defineProperty function without { configurable: true }. jest.spyOn function tries to overwrite existing property but is blocked.

If build using export {}, the result is like below.

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.BatchRequestContent = void 0;
var BatchRequestContent_1 = require("./content/BatchRequestContent");
Object.defineProperty(exports, "BatchRequestContent", { enumerable: true, get: function () { return BatchRequestContent_1.BatchRequestContent; } });

In this case, the Object.defineProperty function will not be called for the property. jest.spyOn function can overwrite existing property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants