[Bug] extended matchers does not work storybook-builder-vite as-is

[bodograumann]

Describe the bug

After adding expect.extend(matchers) in 0.0.7, @storybook/jest stopped working with storybook-builder-vite.

Steps to reproduce the behavior

I have produced a minimal reproduction here:
https://github.com/bodograumann/storybook-jest-vite-interop

• Start the storybook with npm run storybook
• Execute the @storybook/test-runner with npm test

When removing the import { expect } from "@storybook/jest" the test runner works again.
If removing the second story file Button2.stories.json, which does not import from @storybook/jest, the below error message about setMatchers does not appear, but the test runner times out after 15s.

The storybook itself works fine in the browser.

Expected behavior

expect and jest could be imported from @storybook/jest in 0.0.6 just fine.

Logs

> test-storybook --stories-json

TypeError: Object.defineProperty called on non-object                                          
    at Function.defineProperty (<anonymous>)
    at http://localhost:6006/node_modules/.vite-storybook/@storybook_jest.js?v=85d118fd:14613:18
    at Array.forEach (<anonymous>)
    at t2.setMatchers (http://localhost:6006/node_modules/.vite-storybook/@storybook_jest.js?v=85d118fd:14611:21)
    at Function.w.extend (http://localhost:6006/node_modules/.vite-storybook/@storybook_jest.js?v=85d118fd:14477:40)
    at http://localhost:6006/node_modules/.vite-storybook/@storybook_jest.js?v=85d118fd:20032:8

Environment

• OS: Arch Linux
• Node.js version: v17.5.0
• NPM version: 8.5.0
• Browser (if applicable): chromium is used by the test-runner
• Browser version (if applicable): 98.0.4758.102 Arch Linux

[IanVS]

I think I've figured out what is causing this. The import line:

import * as matchers from '@testing-library/jest-dom/matchers';

for some reason I don't quite understand is including a default key, with a value of undefined. So, the matcher here: https://github.com/facebook/jest/blob/d472868ee10eccd6f234ad98f1b76ed20ef4798d/packages/expect/src/jestMatchersObject.ts#L58 is undefined, which blows up on the following line. I hacked in a quick if (matcher) { check, and everything works great. The problem is that this change needs to be made to expect, and then eventually it will get picked up by @storybook/expect, then by @storybook/jest. I'll submit a PR to jest and see if it gets any traction. In the meantime, I'm going to use patch-package to change node_modules/@storybook/expect/dist/esm/index.js

from:

Object.keys(e).forEach((o=>{const i=e[o];if(Object.defineProperty(i,a,{value:t}),!t)

to

Object.keys(e).forEach((o=>{const i=e[o];if(i && Object.defineProperty(i,a,{value:t}),!t)

(note the new i && check)

So far, that seems to be working for me. 🤞

[IanVS]

PR which should solve this problem: jestjs/jest#12481

[bodograumann]

Oh yeah, esm/cjs-compatibility can be a real pain.
This is some weirdness I haven’t encountered before though.

How about working around it here with:

delete matchers.default

[IanVS]

Unfortunately you're not allowed to delete imports like that, you'll get an error. I tried it. 😭

Jest was a bust, they just made it turn into a different error. I've been experimenting with other ways to fix this, most of which have to do with changing how this project is bundled, which I'd need to run past @yannbf. The root cause seems to be the way esbuild processes namespace imports of cjs files.

From https://github.com/evanw/esbuild/blob/master/CHANGELOG.md#0144:

If an import statement is used to load a CommonJS file and a) module.exports is an object, b) module.exports.__esModule is truthy, and c) the file name does not end in either .mjs or .mts and the package.json file does not contain "type": "module", then esbuild will set the default export to module.exports.default (like Babel). Otherwise the default export is set to module.exports (like Node).

Note that this means the default export may now be undefined in situations where it previously wasn't undefined. This matches Webpack's behavior so it should hopefully be more compatible.

Also note that this means import behavior now depends on the file extension and on the contents of package.json. This also matches Webpack's behavior to hopefully improve compatibility.

So, it seems this package would either need to add "type": "module" to its package.json (which does work, I tried it), or the esm export would need to end in .mjs, (which also works). So far I haven't found a way to make typescript create a .mjs file, but maybe a dumb rename step during build would do the trick. Or alternatively we could build with something other than typescript, but that has its own challenges.

[bodograumann]

The best solution is of course only importing from esm dependencies.
Until then, trying to manipulate the behaviour of a specific bundler is not very reliable, imho.

Excuse me if I’m overlooking something trivial, but if we cannot modify the imports (fair enough), how about copying them to a new object?

const validMatchers = { ...matchers };
delete validMatchers.default;
expect.extend(validMatchers);

[IanVS]

I could have sworn I tried that, but yes, that does seem to work. Typescript isn't happy about it in my editor, but it seems to build okay regardless. That might be the simplest solution for now, even though it also seems like a hack to work around bundler issues.

[IanVS]

@yannbf what do you think of @bodograumann's suggestion? Would you accept a PR which adds a delete validMatchers.default?

[yannbf]

Hey @bodograumann thanks for opening this issue! @IanVS I love the movement you set in both jest and testing-library from this issue!

If there's a PR I could spend some time testing out, I trust you @IanVS !

[github-actions]

🚀 Issue was released in v0.0.10 🚀

[yannbf]

Hey @bodograumann thanks once again for opening this issue and @IanVS thank you so much for making the fix! ♥️
Please check https://github.com/storybookjs/jest/releases/tag/v0.0.10 and let us know in case there are any issues.