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.

Sign up for GitHub

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

Jump to bottom

fix: don't throw error for tagName getter in Jest #3269

Closed
wants to merge 1 commit into from
Closed

fix: don't throw error for tagName getter in Jest #3269

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/@lwc/engine-core/src/framework/restrictions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,11 @@ function getCustomElementRestrictionsDescriptors(elm: HTMLElement): PropertyDesc

function getComponentRestrictionsDescriptors(): PropertyDescriptorMap {
assertNotProd(); // this method should never leak to prod
if (process.env.NODE_ENV === 'test') {
Copy link
Contributor

@ravijayaramappa ravijayaramappa Jan 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this change with the reported test case, the new assertion message is

    expect(jest.fn()).not.toBeCalled()

    Expected number of calls: 0
    Received number of calls: 1

    1: {Symbol(@@lockerLiveValue): undefined}

      10 |         document.body.appendChild(element);
      11 |         debugger;
    > 12 |         expect(foo).not.toBeCalled();
         |                         ^
      13 |     });
      14 | });

      at Object.toBeCalled (force-app/main/default/lwc/myComponent/__tests__/index.spec.js:12:25)

The error message isn't much useful to determine that the value is an LWC Component. Jest serializes it like an Object and prints out the enumerable properties. This is acceptable.
However, if we want to make it a bit better, IMO the solution is to provide a serializer for LightningElement component instance via @lwc/jest-serializer and have it call the LightningElement.toString() method

The background is that jest tries to detect the value type. For DOMElement, it uses the tagName property to test if the value is a DOMElement. Hence the error reported by the user. The code is in pretty-format package.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, if we want to make it a bit better, IMO the solution is to provide a serializer for LightningElement component instance via @lwc/jest-serializer and have it call the LightningElement.toString() method

This makes sense; we can open an issue on lwc-test for that.

AIUI, if we did this, then we would not need to add any code to LWC OSS to do special handling for Jest. So yeah, that makes sense to me as an alternative.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

salesforce/lwc-test#168

// In Jest, throwing an error for the tagName breaks certain Jest built-in functionality:
// https://github.com/salesforce/sfdx-lwc-jest/issues/299
return {};
}
return {
tagName: generateAccessorDescriptor({
get(this: LightningElement) {
Expand Down