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

Migration to ES Modules broke Jest unit testing #16

Open
joaosamouco opened this issue Feb 7, 2023 · 4 comments
Open

Migration to ES Modules broke Jest unit testing #16

joaosamouco opened this issue Feb 7, 2023 · 4 comments

Comments

@joaosamouco
Copy link

Describe the bug

The most recent change to this and other packages like kapsule, accessor-fn, float-tooltip, and probably some more, broke my unit-tests pipeline that uses Jest. Jest support for ES Modules is still in experimental phase so things are expected to break.

Details:

    /home/user/code/app/node_modules/float-tooltip/dist/float-tooltip.mjs:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,jest){import { select, pointer } from 'd3-selection';
                                                                                      ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      3 | import React, { useEffect, useRef, useState } from "react";
      4 | import PropTypes from "prop-types";
    > 5 | import Icicle from "icicle-chart";
        | ^
      6 | import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
      7 | import {

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1605:14)
      at Object.<anonymous> (node_modules/icicle-chart/dist/icicle-chart.common.js:12:15)
      at Object.<anonymous> (lib/components/TreeMap/TreeMap.tsx:5:1)
      at Object.<anonymous> (lib/components/TreeMap/TreeMap.stories.tsx:6:1)
      at Object.<anonymous> (lib/components/TreeMap/TreeMap.test.tsx:5:1)

It's okay to migrate to ES Modules but if you are dropping support for CommonJS I believe that the best thing to do is to publish a new major version since this is most likely a breaking change.

Also, if someone knows how to fix this so that Jest is able to process these modules, please let me know.

@vasturiano
Copy link
Owner

@joaosamouco thanks for reaching out.

One of the motivations for the full migration to ES modules is that the CommonJS entrypoint in this package was already not functioning properly, because some of the sub-dependencies were already not CommonJS compatible; such as d3-scale for example.

Thus, it wasn't a major version bump because in fact the feature was already broken, and the package was falsely advertising that it could be safely bound as a CommonJS module.

In your case I'd suggest pinning to v1.6.0 if you really need the explicit CommonJS binding. But the future-proof way is to migrate to using ES modules.

Is the Jest (experimental) ESM support not functioning well enough in this case?

@joaosamouco
Copy link
Author

Hi!

In your case I'd suggest pinning to v1.6.0 if you really need the explicit CommonJS binding. But the future-proof way is to migrate to using ES modules.

This won't work since icicle-chart is not using fixed dependencies versions so even older versions of icicle-chart will still install more recent versions of kapsule and float-tooltip for example. Only a major version would prevent this from happening. (I'm not using package-lock.json to lock all the dependencies so that's also my bad.

Is the Jest (experimental) ESM support not functioning well enough in this case?

After some trial and error I was finally to make it work, I will leave the instructions in case it is helpful to someone else.

  1. Install @babel/core, babel-jest and @babel/plugin-transform-modules-commonjs
npm i -D @babel/core babel-jest @babel/plugin-transform-modules-commonjs
  1. Update or create a babel.config.js file to use this transformer only when testing
module.exports = {
  env: {
    test: {
      plugins: ['@babel/plugin-transform-modules-commonjs'],
    },
  },
};
  1. Update jest.config.js to transform icicle-chart, accessor-fn, kapsule, and float-tooltip
// jest.config.js
module.exports = () => ({
  ...
  transform: {
    ...
    // support for ES Modules
    'node_modules/(float-tooltip|kapsule|icicle-chart|accessor-fn)/.+\\.m?jsx?$': 'babel-jest',
  },
  // ignore all node_modules except the ones we are transforming
  transformIgnorePatterns: [
    'node_modules/(?!(float-tooltip|kapsule|icicle-chart|accessor-fn)/)',
  ],
});

@vasturiano
Copy link
Owner

@joaosamouco I'm surprised you didn't already have this issue with all the d3-* dependencies. Their latest major version is also ESM only. Did you not have to include them as exceptions in your Jest config?

@joaosamouco
Copy link
Author

@joaosamouco I'm surprised you didn't already have this issue with all the d3-* dependencies. Their latest major version is also ESM only. Did you not have to include them as exceptions in your Jest config?

Oh, I see. For d3-* I had a different solution based on this issue jestjs/jest#12036. The same solution could also work in this case.

// jest.config.js
module.exports = () => ({
  ...
  moduleNameMapper: {
    ...,
    '^d3-(.*)$':
      '<rootDir>/node_modules/d3-$1/dist/d3-$1.min.js'
    '^(icicle-chart|float-tooltip|kapsule|accessor-fn)$':
      '<rootDir>/node_modules/$1/dist/$1.min.js',
  },
});

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

No branches or pull requests

2 participants