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

Can't use v8 in Jest unit tests #246

Closed
screendriver opened this issue Mar 6, 2018 · 10 comments
Closed

Can't use v8 in Jest unit tests #246

screendriver opened this issue Mar 6, 2018 · 10 comments

Comments

@screendriver
Copy link

I am using TypeScript, Jest and Webpack in my project. I upgraded to v8 and now I can't use react-waypoint in my unit tests anymore.

With v7 I imported this library with

import * as Waypoint from 'react-waypoint';

Now with v8 I have to import it like

import Waypoint from 'react-waypoint';

With that default import it runs with TypeScript and Webpack as expected but not in my unit tests that run with Jest. If I change the import to the v7 import * as Waypoint syntax TypeScript and Webpack fails, but Jest runs fine.

I believe it has something to do how this library is exported in the package because in Jest I transpile everything to CommonJS (because Node only understands CommonJS) but then there is a .default import which doesn't exist. In Webpack I transpile everything to ESM import syntax and TypeScript take the default export and Webpack can handle that.

My Workaround at the moment is to use v7 instead of v8 because there everything works out of the box.

Maybe this is related to #238? 🤔

@trotzig
Copy link
Collaborator

trotzig commented Mar 6, 2018

That's unfortunate. Would you mind sharing your babel config (.babelrc or similar)? There might be a tweak you can make to the babel config in order to work around this issue.

Given the amount of churn I've seen with providing both a commonjs and an es module, I wonder if the multi-module approach is leading to more trouble than it's worth? Does @lencioni or @jamesplease have any input here around how important the es module is? I'm considering a revert back to commonjs until tooling support is improved (jest, webpack, etc). But I'd like to hear from you first. 🙏

@screendriver
Copy link
Author

screendriver commented Mar 6, 2018

Would you mind sharing your babel config (.babelrc or similar)

As I said I'm using TypeScript. But I could share my tsconfig.json files if you want?

Multi-Module packaging should not be a problem as of today. For example you can look at other libraries like redux ore something else how they do accomplish that 😉

@trotzig
Copy link
Collaborator

trotzig commented Mar 6, 2018

As I said I'm using TypeScript. But I could share my tsconfig.json files if you want?

Go ahead 👍

It looks like redux is only using named exports, so they might have dodged the bullet.

@screendriver
Copy link
Author

Default exports are doomed 😉 In our project default exports are forbidden because of different reasons 😊

Would it be a good idea to change your exports to named exports and get rid of default exports completely?

@jamesplease
Copy link
Collaborator

@trotzig I think it’s fine to drop the ES module, just because there’s currently no value in providing it. It is really only useful for tree shaking, but there is only one export here, so there is nothing to shake.

In regards to named vs default exports, the only interop issue between these should be when you use them both at the same time. We are only using default exports here, so any issues should be with an individual developer’s setup.

In our project default exports are forbidden because of different reasons

It sounds to me like this issue stems from however your project is set up. I use Jest with named and default exports without an issue.


tl;dr I don’t think we should remove the ES module build because of this particular issue (I’d probably just close this out until there’s more evidence that this is typical of TypeScript projects), but I’d be alright removing it for other reasons.

@screendriver
Copy link
Author

It sounds to me like this issue stems from however your project is set up

Not really. We don't have any technical issues. We just dropped support for default exports because they can be difficult to maintain, refactor and their semantic could change without knowing it. Furthermore you don't know what you really import. For example import foo from 'mylib' compared to import { calculateDistance } from 'mylib'. And we are not alone with this opinion 😉

But this has nothing to do with this issue. When I import react-waypoint 8.0.1 in my TypeScript project like that

import Waypoint from 'react-waypoint';

console.log(Waypoint);

It gives me the component when I bundle everything with webpack but undefined when I run my unit tests with Jest.

After a little bit debugging I believe I found out what's going on: in your package.json you have

"main": "cjs/index.js",
"module": "es/index.js",

defined. That's cool. Webpack respects that and uses es/index.js. The TypeScript definition is also for the ESM file.
Now when I run my tests with Jest, Jest uses cjs/index.js instead of es/index.js. What now happens is that TypeScript adds a .default:

TypeScript

import Waypoint from 'react-waypoint';
console.log(Waypoint);

gets transpiled to

"use strict";
exports.__esModule = true;
var react_waypoint_1 = require("react-waypoint");
console.log(react_waypoint_1["default"]);

But there is no .default. If you had named exports it would look like this:

import * as Waypoint from 'react-waypoint';
console.log(Waypoint);

gets transpiled to

"use strict";
exports.__esModule = true;
var Waypoint = require("react-waypoint");
console.log(Waypoint);

and then it should work. I also found some issues / PR's in the Jest repository like this jestjs/jest#4842 or that jestjs/jest#5485

So I found the problem but don't have a solution 😁 Except "change your default export to a named export" or "only use CommonJS for your package".

@andrew-me
Copy link

Thanks for the explanation.

I got around this by creating a jest mock of waypoint that exports default.

So in __mocks__ I created a file react-waypoint.js with

const Waypoint = require("react-waypoint");

exports.default = Waypoint;

Bit dirty, but does the job for now

@MatthewHerbst
Copy link
Contributor

@trotzig would there be any harm in providing both the default export and a named export?

@trotzig
Copy link
Collaborator

trotzig commented Jun 7, 2018

It doesn't feel like the right solution to the problem. The harm with two exports is that people get confused about what the difference is between the two. That being said, I don't have a good solution to the original problem other than ditching the es module completely and just using a commonjs module.

@screendriver
Copy link
Author

I fixed it by myself with introducing two compiler flags to my project: esModuleInterop and allowSyntheticDefaultImports. I change every import from import * as foo from 'foo'; to import foo from 'foo'; and now it works as expected.

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

5 participants