omgovich / react-colorful

🎨 A tiny (2,8 KB) color picker component for React and Preact apps
https://omgovich.github.io/react-colorful
MIT License
3.19k stars 100 forks source link

Jest 28 with jsdom environment can't parse an exported file from react-colorful #190

Closed NikaBuligini closed 2 years ago

NikaBuligini commented 2 years ago

Recently I upgraded jest from v27 to v28 in my app. I noticed that tests that import react-colorful started to fail with the following error message: SyntaxError: Cannot use import statement outside a module.

node: 16.14.0 jest: 28.1.3 jest-environment-jsdom: 28.1.3 react-colorful: 5.5.1

I investigated it a bit and I found a reason. It's happening because jsdom environment can't parse the file. Here is how the conditional exports for react-colorful look like:

https://github.com/omgovich/react-colorful/blob/e7085123c627f4b5f150bd9973805d8b357f519a/package.json#L11-L19

As you can see for the browser you export ./dist/index.module.js that is not CommonJS. If I change it to ./dist/index.js jest is able to handle it.

How to reproduce

package.json

{
  "name": "test-react-colorful",
  "version": "0.0.0",
  "private": true,
  "scripts": {
    "test": "jest . --env=jsdom"
  },
  "devDependencies": {
    "@babel/preset-env": "^7.18.10",
    "@babel/preset-typescript": "^7.18.6",
    "@types/jest": "^28.1.6",
    "babel-jest": "^28.1.3",
    "jest": "^28.1.3",
    "jest-environment-jsdom": "^28.1.3"
  },
  "dependencies": {
    "react": "^18.2.0",
    "react-colorful": "^5.5.1"
  }
}

babel.config.js

module.exports = {
  presets: [
    ["@babel/preset-env", { targets: { node: "current" } }],
    "@babel/preset-typescript",
  ],
};

component.ts

import { HexColorPicker } from 'react-colorful';

export const Component = HexColorPicker;

component.test.ts

import { Component } from './component';

describe('Component', () => {
    it('should pass', () => {
        expect(Component).not.toBe(null);
    })
})

I can create a pull request to export CommonJS for browsers if you don't mind

omgovich commented 2 years ago

Hi! Thanks for the issue.

AFAIK exporting CJS code for the browser environment is a bad idea and that could break lots of projects using react-colorful.

Browsers can't handle module.exports / require() constructions, so even if it would fixes your issue, it will break the package.

Actually, it's the first time we get an issue with Jest. We use the same approach as Preact does in their bundling and package.json and it always worked well: https://github.com/preactjs/preact/blob/master/package.json

NikaBuligini commented 2 years ago

Hi! Thanks for the reply.

It makes sense. I shouldn't have suggested using CommonJS for browsers.

Everything worked well in Jest 27. I tried downgrading it and the test works fine (even with jsdom environment).

Do you really need to specify export for "browser"? I'm looking at https://github.com/pmndrs/zustand/blob/main/package.json#L23 and I don't see them having a separate export for the browser. Removing it from the package fixes my issue.

I don't insist on any changes in the library. I'm just trying to figure things out.

PetroSilenius commented 2 years ago

Hey! I'm facing the same exact issue in a project and I would also recommend removing the "browser" export

There's a pretty good description on @hookform/resolvers on how they dealt with this The issue https://github.com/react-hook-form/resolvers/issues/396 The pull request https://github.com/react-hook-form/resolvers/pull/435

omgovich commented 2 years ago

Hey guys! Thanks for your suggestions! Makes sense to me: I'll try removing the browser field then.

omgovich commented 2 years ago

Just released v5.6.1. Could you please try it in your projects?

PetroSilenius commented 2 years ago

Works wonderfully, thanks for the quick change! 🙏🏻

NikaBuligini commented 2 years ago

It works for me too. Thanks!