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

TypeError: mappedCoverage.addStatement is not a function #211

Closed
screendriver opened this issue May 12, 2017 · 37 comments
Closed

TypeError: mappedCoverage.addStatement is not a function #211

screendriver opened this issue May 12, 2017 · 37 comments

Comments

@screendriver
Copy link
Contributor

  • Issue

I updated my project to Jest 20.0.1 and updated ts-jest to 20.0.3 as well. I tried out the new coverage feature and get following error:

UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): TypeError: mappedCoverage.addStatement is not a function
  • Expected behavior

Coverage should run.

  • my config
"jest": {
    "globals": {
      "__TS_CONFIG__": "./test/tsconfig.json"
    },
    "transform": {
      ".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
    },
    "testRegex": "(/__tests__/.*|\\.(test|spec))\\.(ts|tsx|js)$",
    "testPathIgnorePatterns": [
      "<rootDir>/build"
    ],
    "snapshotSerializers": [
      "enzyme-to-json/serializer"
    ],
    "setupTestFrameworkScriptFile": "./test/setupTestFramework.ts",
    "mapCoverage": true,
    "moduleFileExtensions": [
      "ts",
      "tsx",
      "js"
    ],
    "moduleNameMapper": {
      "\\.(svg|woff|ttf|eot)$": "<rootDir>/test/__mocks__/fileMock.js",
      "\\.(css)$": "<rootDir>/test/__mocks__/styleMock.js"
    },
    "setupFiles": [
      "./test/setupJest.ts"
    ]
  },
@mrijke
Copy link

mrijke commented May 12, 2017

This issue can be reproduced in my test repo as well, see mrijke/react-typescript-jest-demo#1

@kulshekhar
Copy link
Owner

@mrijke thanks. Can you update the repo to use the latest versions of jest & ts-jest?

@mrijke
Copy link

mrijke commented May 12, 2017

@kulshekhar as noted in mrijke/react-typescript-jest-demo#1 (comment), I've updated to ts-jest 20.0.3 and jest 20.0.1, which seem to be the latest versions available and the issue persists.

@screendriver
Copy link
Contributor Author

I don't know if this is really ts-jest or jest related? 🤔

@kulshekhar
Copy link
Owner

@screendriver to find that out, we'll need a minimal repo that reproduces the error you're getting :)

The other linked repo isn't doing that

@mrijke
Copy link

mrijke commented May 12, 2017

I have merged the PR now that updates the versions of ts-jest to 20.0.3 and jest to 20.0.1, so now it should be a proper repro repo. Or is something else missing?

@screendriver
Copy link
Contributor Author

The other linked repo isn't doing that

Ah ok. I didn't knew that. I thought that it does exactly that.

I have merged the PR now that updates the versions of ts-jest to 20.0.3 and jest to 20.0.1, so now it should be a proper repro repo.

Great 👍

@kulshekhar
Copy link
Owner

I tested the updated repo which reproduced this issue.

The tests execute fine and it seems that the error is thrown from jest (at the stage when coverage is being generated).

That said, I suspect the cause of the error might not be in jest but in the configuration (package.json and/or .babelrc). I don't use babel so I might not be the best person to comment/speculate on this.

We can leave this issue open if you want to continue the discussion/collaboration here. Hopefully someone familiar with babel/jest will be able to point out a solution.

@screendriver
Copy link
Contributor Author

Should I open the same issue at the Jest repo and reference this one?

@kulshekhar
Copy link
Owner

@screendriver if this error can be reproduced without using ts-jest, it might be a good idea. Without that, I'm not sure how the folks at jest will be able to help.

@screendriver
Copy link
Contributor Author

Hmmm to be honest: I don't know. The new official Jest docs mention TypeScript for the new coverage feature 🤔

@kulshekhar
Copy link
Owner

I guess might be worth a shot

@screendriver
Copy link
Contributor Author

Done: jestjs/jest#3560

@jrwebdev
Copy link

I had this issue at work last week, but being relatively new to TypeScript I assumed it was some form of configuration I'd set incorrectly. I didn't find the problem, but I traced it down to istanbul-lib-source-maps - it appears that it's unable to find a source map, so it just uses a file coverage object for the given file:

https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-source-maps/lib/transformer.js#L161

Then due to this it doesn't instantiate MappedCoverage:

https://github.com/istanbuljs/istanbuljs/blob/master/packages/istanbul-lib-source-maps/lib/transformer.js#L152

This is when I assumed I'd screwed up generating source maps and went back to experimenting with that, but didn't get any further before I left for the weekend. Only thing I really tried doing was using inlineSourceMap instead of sourceMap.

The only other bits of info I can give at the moment that may help is that I'm also pretty sure if I put in a check for addStatement being available before it is called (along with addFunction and addBranch which also are missing), it did go ahead and generate everything successfully as far as I could see. I also had it working on a branch that just contained a couple of tests for very basic functions in .ts files, but when .tsx files and Enzyme were threw into the mix as well that's when the issues started.

@jrwebdev
Copy link

jrwebdev commented May 15, 2017

I've had another look into this - the issue (at least with my config) is when allowSyntheticDefaultImports is set to true. If set, the preprocessor runs files through babel-jest and appends a .js extension onto them. When the coverage is generated, both the .ts / .tsx and .ts.js / .tsx.js files are run through the coverage generator, but source maps are only found for .ts.js / .tsx.js files, so the original files encounter the problems I described above with missing source maps.

I'm afraid this is about as far as I can take it in terms of a fix, but I hope the above info is helpful to whoever picks this up.

@ecozoic
Copy link

ecozoic commented May 17, 2017

Removing allowSyntheticDefaultImports didn't seem to fix it for me, but it definitely seems .tsx file related. If I run my tests only on .ts files, I get coverage reports no problem. Once you put .tsx files in the mix (via testRegex), I get the error. I'm able to specify tsx files in the collegeCoverageFrom and they will show up in the report (0% coverage), but executing the tsx file tests is what causes the error.

tsconfig.json

{
  "compilerOptions": {
    "target": "es5",
    "module": "es2015",
    "moduleResolution": "node",
    "rootDir": "src/client",
    "outDir": "dist",
    "sourceMap": true,
    "lib": ["es2015", "dom"],
    "jsx": "react",
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noUnusedLocals": true,
    "suppressImplicitAnyIndexErrors": true
  }
}

jest.conf.json

{
  "collectCoverage": false,
  "collectCoverageFrom": ["src/client/app/**/*.{ts,tsx}"],
  "coverageDirectory": "<rootDir>/coverage/client",
  "coverageReporters": ["json", "lcov"],
  "mapCoverage": true,
  "moduleFileExtensions": [
    "ts",
    "tsx",
    "js"
  ],
  "moduleNameMapper": {
    "\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga|ico)$": "<rootDir>/jest/file-mock.js",
    "\\.(css|less|sass|scss|styl)$": "identity-obj-proxy"
  },
  "roots": ["src/client"],
  "snapshotSerializers": ["enzyme-to-json/serializer"],
  "testEnvironment": "jsdom",
  "testRegex": "(/__tests__/.*|\\.(test|spec))\\.(ts|tsx)$",
  "transform": {
    ".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
  }
}

@kulshekhar
Copy link
Owner

kulshekhar commented May 17, 2017

Based on the observation by @ecozoic I have a hunch of what's going on. When allowSyntheticDefaultImports is used, this line adds a .js extension to the file. Maybe it needs to add the extension (js/jsx) based on the type of the source file (ts/tsx)

I'll try to check this but if someone can try this out in the meanwhile, it might lead to a quicker resolution

@ecozoic
Copy link

ecozoic commented May 17, 2017

To clarify, I get this error whether or not I use allowSyntheticDefaultImports

Repo for repro here: https://github.com/ecozoic/react-starter/tree/typescript-2

UPDATE: Deff seems to be something w/ ts-jest, I swapped out the ts-jest preprocessor for a simple one based on the official examples and I'm able to get coverage reports working (although it doesn't look like remapping is working...).

const tsc = require('typescript');
const tsConfig = require('../tsconfig.json');

tsConfig.compilerOptions.module = 'commonjs';
tsConfig.compilerOptions.inlineSourceMap = true;
tsConfig.compilerOptions.inlineSources = true;

delete tsConfig.compilerOptions.sourceMap;
delete tsConfig.compilerOptions.outDir;

module.exports = {
  process(src, path) {
    if (path.endsWith('.ts') || path.endsWith('.tsx')) {
      return tsc.transpile(src, tsConfig.compilerOptions, path, []);
    }
    return src;
  },
};

UPDATE 2: Clearing the Jest cache fixed the mapping issues. So that code snippet seems to work.

@screendriver
Copy link
Contributor Author

Just for your information regarding allowSyntheticDefaultImports: I have to use it in my tests because of #182

@GeeWee
Copy link
Collaborator

GeeWee commented May 21, 2017

This happens only when the output is piped through babel-jest. I'm not sure why it happens. I tried fixing the file extensions as suggested by @kulshekhar.
Might have to dig into the source of babel-jest for this one.

@GeeWee
Copy link
Collaborator

GeeWee commented May 21, 2017

@screendriver @ecozoic @mrijke
Can I get any of you to try and manually go in and edit your node_modules/ts-jest/dist/preprocessor.js and replace line 31-33
aka

        var outputText = compilerOptions.allowSyntheticDefaultImports
            ? babelJest.process(tsTranspiled.outputText, path + '.js', config, transformOptions)
            : tsTranspiled.outputText;

with:

        var outputText = compilerOptions.allowSyntheticDefaultImports
            ? babelJest.process(tsTranspiled.outputText, path + '.js', config, null)
            : tsTranspiled.outputText;

to see if this resolves the issue?

@screendriver
Copy link
Contributor Author

It works! But to be honest: I did an update on all of mine dependencies, deleted my node_modules directory and reinstalled everything. After that it is still working 😳 Has something changed in the last days?

@lukescott
Copy link

lukescott commented May 22, 2017

I think it has to do with the missing canInstrument: true property. babel-jest has this, and then in jest-runtime it checks for this. If it is missing, it will run it thru babel again and do another transform with babel & istanbul. Still don't have it working yet though.

@GeeWee
Copy link
Collaborator

GeeWee commented May 23, 2017

@screendriver - I think that might be a jest cache issue - try running it with --no-cache. It works in my tests, however the coverage numbers are a little off from earlier, and I haven't had time to look into what is correct.
@jrwebdev - Interesting, I think I'll try monkey patching babel-jest and see if you're right.
@lukescott - interesting. Do you have some code locations you can share where you see this?

@lukescott
Copy link

I updated issue #230. I initially posted here because I thought it was the same issue, but it may not be.

@Apidcloud
Copy link

Apidcloud commented May 25, 2017

I was facing the original mapCoverage issue, as well as getting reports with the .ts.js files, but removing allowSyntheticDefaultImports from tsconfig and setting --no-cache on jest flag solved both issues.

It's obviously slower generating the reports though.

Can someone show a simple example of the need of babel during this process? I'm just using ts-jest for the tests, and they seem to be working well, even with async/await and Promises.

I have also tried typescript-babel-jest preprocessor, and the map coverage is actually worse (it might be related to the fact that it's a really simple one). Other than that, seems to be all the same.

@kulshekhar
Copy link
Owner

@Apidcloud babel is used only when allowSyntheticDefaultImports is set to true. The PR for that was accepted in response to issues that users were facing. All these users needed to use babel for one reason or another.

Now, if you need to use allowSyntheticDefaultImports and don't want to use babel, you can set skipBabel to true in package.json jest > globals > ts-jest > skipBabel

@GeeWee
Copy link
Collaborator

GeeWee commented May 25, 2017

Hit the nail right on the head @jrwebdev - patching babel-jest to not only transpile .js files, so we could pass the correct path to it worked, both with coveragemaps and coverage. (Oddly enough, I am getting a one-off error in the #statements in coverage, but after looking at the coverage maps I think this might be more accurate)

I think we should consider maintaining our own fork of babel-jest, it's a very small program and the other solution is to monkey-babel, and that seems like it'll end terribly. I doubt they'll accept a fix in their repo as this falls far outside their usecases.

What do you think about that @kulshekhar

@kulshekhar
Copy link
Owner

kulshekhar commented May 25, 2017

I'm not really in favor of maintaining a fork of anything - over a period of time, they tend to diverge and cause real maintenance issues.

That said, if this seems like a workable solution, maybe someone can create a separate ts-babel-jest (or something on those lines). ts-jest could then have a configuration (for cases when babel is needed) so that the user can choose from either the standard babel-jest or ts-babel-jest. This way we keep the issues arising from divergence between babel-jest and the fork out of ts-jest

@GeeWee
Copy link
Collaborator

GeeWee commented May 25, 2017 via email

@kulshekhar
Copy link
Owner

kulshekhar commented May 25, 2017

Alternatively, we could just create a hook that can be called if provided. This would be after typescript transpilation and would supply the specified function the transpiled code as well as all the parameters that the process function receives from jest.

This would be a lot more flexible. ts-jest can come with a default babel setup and those who want something different can override this hook and get the functionality they want.

Edit: forgot to address this. babel-jest might be short but I don't doubt there'll be babel specific requirements that will expand our version (should there be one) to something that's completely different from the original one.

We should make it easy for users to use another transformer if they want to but we should limit the scope of ts-jest to only typescript specific issues and features.

@GeeWee
Copy link
Collaborator

GeeWee commented May 25, 2017

You mean let people pipe it through babel themselves?
I agree this would be more flexible, however no matter what, we IMO want to pipe our output through babel, if nothing else but to get hoists #227 to work.
I feel like piping tsc to babel and getting it working is obviously reasonably difficult, seeing all the edge cases we have to cover, I'd hate if we had to put this complexity on the shoulders of the users.

I feel like we need to:

  • Pipe all ts code through babel to get the hoist calls to work as expected
  • Aditionally allow for module transpilation if syntheticdefaults is set to true.

I feel like anything more complex than that, should not be handled by us, but by someone making a seperate transformer where they pipe the output through ts-loader and whatever else they want themselves.

@kulshekhar
Copy link
Owner

we IMO want to pipe our output through babel, if nothing else but to get hoists #227 to work.

The default hook I mentioned can do this.

I feel like piping tsc to babel and getting it working is obviously reasonably difficult, seeing all the edge cases we have to cover, I'd hate if we had to put this complexity on the shoulders of the users.

What are 'edge cases' to us maintaining ts-jest is really the 'main case' for users encountering them. No one but these users are best placed to address these issues. What we should focus on, imo, is making it straightforward for users to tune ts-jest to suit their needs.

With that in mind, here's how I think ts-jest needs to evolve:

  • remove all special handling of synthetic defaults (or anything else). Just process the source code according to the tsconfig settings
  • create a hook that allows users to pass the output of tsc through any function they provide. The default version of this hook can use babel to address Jest.mock calls are not hoisted to the top #227

Apologies to all those subscribed to this issue for all the noise. We really should take this discussion over to a new issue :)

@GeeWee
Copy link
Collaborator

GeeWee commented May 25, 2017

Yes I agree - let's take it to #235 - I think we might need to resolve that before we can do real work on this issue

This was referenced May 26, 2017
@kulshekhar
Copy link
Owner

I just tested this and it looks like #236 fixed it.

@screendriver @mrijke can you check if this works for you

@screendriver
Copy link
Contributor Author

LGTM 👍

@GeeWee
Copy link
Collaborator

GeeWee commented May 30, 2017

Awesome!

rmeritz added a commit to bocoup/airtable.js that referenced this issue Jun 30, 2020
- Change the path of the required files in the tests to test the build
files
- Change the test runner to compile the typescript prior to running the
tests
- Change the test runner to only run the tests once (coverage also runs
the tests)
- Pass --no-cache to coverage to ensure it works with typescript:
kulshekhar/ts-jest#211
- Stop using `grunt-ts` because it doesn't actually just pass the config
through tsc. This caused problems when the new resolveJsonModule config
didn't get passed through. Instead just use `tsc` to compile directly.
- Modify the `tsconfig` to ensure that json files are included in the
compiled package.
- One line of code is no longer coverage for mysterious reasons, punt on
figuring out why and just ignore it so they test continue to pass.
mzgoddard pushed a commit to bocoup/airtable.js that referenced this issue Jul 28, 2020
- Change the path of the required files in the tests to test the build
  files
- Change the test runner to compile the typescript prior to running the
  tests
- Change the test runner to only run the tests once (coverage also runs
  the tests)
- Pass --no-cache to coverage to ensure it works with typescript:
  kulshekhar/ts-jest#211
- Stop using `grunt-ts` because it doesn't actually just pass the config
  through tsc. This caused problems when the new resolveJsonModule
  config didn't get passed through. Instead just use `tsc` to compile
  directly.
- Modify the `tsconfig` to ensure that json files are included in the
  compiled package.
- One line of code is no longer coverage for mysterious reasons, punt on
  figuring out why and just ignore it so they test continue to pass.
mzgoddard pushed a commit to bocoup/airtable.js that referenced this issue Aug 10, 2020
- Change the path of the required files in the tests to test the build
  files
- Change the test runner to compile the typescript prior to running the
  tests
- Change the test runner to only run the tests once (coverage also runs
  the tests)
- Pass --no-cache to coverage to ensure it works with typescript:
  kulshekhar/ts-jest#211
- Stop using `grunt-ts` because it doesn't actually just pass the config
  through tsc. This caused problems when the new resolveJsonModule
  config didn't get passed through. Instead just use `tsc` to compile
  directly.
- Modify the `tsconfig` to ensure that json files are included in the
  compiled package.
- One line of code is no longer coverage for mysterious reasons, punt on
  figuring out why and just ignore it so they test continue to pass.
rwaldron pushed a commit to bocoup/airtable.js that referenced this issue Sep 1, 2020
- Change the path of the required files in the tests to test the build
  files
- Change the test runner to compile the typescript prior to running the
  tests
- Change the test runner to only run the tests once (coverage also runs
  the tests)
- Pass --no-cache to coverage to ensure it works with typescript:
  kulshekhar/ts-jest#211
- Stop using `grunt-ts` because it doesn't actually just pass the config
  through tsc. This caused problems when the new resolveJsonModule
  config didn't get passed through. Instead just use `tsc` to compile
  directly.
- Modify the `tsconfig` to ensure that json files are included in the
  compiled package.
- One line of code is no longer coverage for mysterious reasons, punt on
  figuring out why and just ignore it so they test continue to pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants