webpack-contrib/istanbul-instrumenter-loader

Upgrading from babel-istanbul-loader, build fails with JSX

ssylvia opened this issue ยท 25 comments

ssylvia commented

I saw that babel-istanbul-loader was deprecated and was pointed to this library as the upgrade. My webpack now fails to build if there is JSX syntax in code.

Does istanbul-instrumenter-loader support react JSX? Do I need any other changes?

Previous config from babel-istanbul-loader:

module: {
      rules: [
        {
          include: [
            path.resolve(__dirname, './src'),
          ],
          test: /\.(js|jsx)$/,
          use: [
            {
              loader: 'babel-loader',
              options: {
                cacheDirectory: true,
              },
            },
          ],
        },
        {
          enforce: 'pre',
          exclude: /\.test\.js|jsx$/,
          include: [
            path.resolve(__dirname, './src'),
          ],
          test: /\.(js|jsx)$/,
          use: [
            {
              loader: 'babel-istanbul-loader',
              options: {
                cacheDirectory: true,
              },
            },
          ],
        },

new config with istanbul-instrumenter-loader:

module: {
      rules: [
        {
          include: [
            path.resolve(__dirname, './src'),
          ],
          test: /\.(js|jsx)$/,
          use: [
            {
              loader: 'babel-loader',
              options: {
                cacheDirectory: true,
              },
            },
          ],
        },
        {
          exclude: /\.test\.js|jsx$/,
          include: [
            path.resolve(__dirname, './src'),
          ],
          test: /\.(js|jsx)$/,
          use: [
            {
              loader: 'istanbul-instrumenter-loader',
              options: {
                esModules: true,
              },
            },
          ],
        },

.babelrc

{
  "plugins": ["dynamic-import-webpack"],
  "presets": [
    "latest",
    "react",
    "stage-3"
  ],
  "env": {
    "test": {
      "plugins": ["istanbul"]
    }
  }
}
deepsweet commented

really weird, it should use the same Babel with .babelrc under the hood... could you please show me the exactly error message?

ssylvia commented

@deepsweet Here's the error I get:

webpack: wait until bundle finished:

ERROR in ./src/app/index.js
Module build failed: SyntaxError: Unexpected token (11:2)
    at Parser.pp$5.raise (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:4333:13)
    at Parser.pp.unexpected (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:1705:8)
    at Parser.pp$3.parseExprAtom (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3670:12)
    at Parser.pp$3.parseExprSubscripts (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3414:19)
    at Parser.pp$3.parseMaybeUnary (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3394:19)
    at Parser.pp$3.parseExprOps (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3324:19)
    at Parser.pp$3.parseMaybeConditional (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3301:19)
    at Parser.pp$3.parseMaybeAssign (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3264:19)
    at Parser.pp$3.parseExprListItem (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:4190:16)
    at Parser.pp$3.parseCallExpressionArguments (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3493:20)
    at Parser.pp$3.parseSubscripts (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3453:31)
    at Parser.pp$3.parseExprSubscripts (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3424:15)
    at Parser.pp$3.parseMaybeUnary (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3394:19)
    at Parser.pp$3.parseExprOps (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3324:19)
    at Parser.pp$3.parseMaybeConditional (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3301:19)
    at Parser.pp$3.parseMaybeAssign (/Users/myUserName/appRepo/node_modules/babylon/lib/index.js:3264:19)
webpack: Failed to compile.

This is my ./src/app/index.js file:

import 'babel-polyfill';
import App from './app.jsx';
import AppStore from './store/AppStore';
import {Provider} from 'react-redux';
import React from 'react';
import {render} from 'react-dom';

const appStore = new AppStore();

render(
  <Provider store={appStore}>
    <App />
  </Provider>, document.getElementById('app')
);
  • ๐Ÿ‘8
bencripps commented

Having this exact issue as well. I've got the "react" preset listed in my .bablrc and I've got the following loader attached:

{
        test: /\.js$|\.jsx$/,
        enforce: 'pre',
        exclude: /node_modules|test/,
        loader: 'istanbul-instrumenter-loader',
        query: {
            esModules: true,
            presets: ['react']
        }
    }
bencripps commented

Any idea on this one? I'd like to upgrade to webpack2, but I don't wanna lose code coverage.

deepsweet commented

looks like there should additional plugins: [ 'jsx', 'flow' ] option here โ€“ babelrc has nothing to do with Babylon.

bencripps commented

Since babylon doesnt expect plugins or presets, doesn't it seem like the instrumenter is expecting already-transpiled code?

How did this work before with JSX/ES6? Was the code already transpiled before it got to the instrumenter?

deepsweet commented

this one didn't work before with JSX/ES6 :) that's why we had babel-istanbul-loader and isparta-loader.

bencripps commented

Hmm, what should we do now since both those packages have been deprecated?

deepsweet commented

they are deprecated because now we finally have "an official" solutiuon, new Istanbul integrated with Babel, but code coverage is a forever pita ยฏ\_(ใƒ„)_/ยฏ try https://github.com/istanbuljs/babel-plugin-istanbul

I personally have no idea how do they want to transpile it directly with Babylon and allow you to use any non-default transforms like JSX, Flow, etc at the same time.

  • ๐Ÿ‘1
bencripps commented

I'll give that a shot, thanks!

unimonkiez commented

@bencripps How did it go?

bencripps commented

Success! Here is the changeset.

The issue was that, previously, I had used the istanbul loader in the pre stage -- rather than transpile the code before it gets to the pre stage, I am now instrumenting the code after the loaders take effect.

Here's a post about the upgrade.

  • ๐Ÿ‘1
Axbon commented

I have tried this and this does not work well for us. This used to work with webpack 1 and the old "loader: 'babel-istanbul". We used this in the "preloader" stage, which correctly instrumented the es6/jsx code and coverage was correct. After upgrading to webpack 2 and now using the istanbul-instrumenter-loader it only works in the "post" stage, which to me seems incorrect. It's not desired to get coverage on transpiled code, since this code is not the original, its created by babel. If used in the "pre" stage it gets the error Unexpected Token as if it doesnt support JSX - but how did babel-instanbul support that? Whats the major difference here?

I am also curious why it seems to be working for bencripps, I mean... is your coverage really correct, on the transpiled code? Do you get undefined line numbers in the report?

Axbon commented

I solved the problem regarding jsx support in pre-stage in istanbul-lib-instrument, will fork or try to get a pr through there.

glenpike commented

Sorry to butt in on this tthread - my question is for @Axbon
I tried using your fork instead with the following setup in package.json

"istanbul-instrumenter-loader": "git://github.com/Axbon/istanbul-instrumenter-loader.git#87f7ede",

But npm fails with:

npm ERR! git rev-list -n1 1.4.2: fatal: ambiguous argument '1.4.2': unknown revision or path not in the working tree.

I might be wrong, but I think you might need to point your temp dependency url for istanbul-lib-instrument to your commit #c50e2cb rather than 1.4.2? e.g.

"istanbul-lib-instrument": "git://github.com/Axbon/istanbul-lib-instrument.git#c50e2cb",
Axbon commented

Ah true, anyway that fork was a temporary thing just to get JSX support added. I am happy to report that it has now landed in istanbul-lib-instrument@1.7.0 , perhaps @deepsweet could update the dependency so that jsx works, otherwise happy to send pr :)

kulakshay commented

Hey @Axbon, I tried using istanbul-lib-instrument@1.7.0 still not working for jsx code.
I tried in both pre and post modes.

Axbon commented

@kulakshay seems like they made a change and moved the istanbul-lib-instrument, it now says: deprecated, instead use https://github.com/istanbuljs/istanbuljs so I suppose the dependency has to be updated there, and I verified that the jsx support is indeed merged into that codebase, not the other one however. Sorry for the confusion! The code can be found here: https://github.com/istanbuljs/istanbuljs/tree/master/packages/istanbul-lib-instrument and I guess this repo has to be updated, so that the instrumenter is imported from istanbuljs instead.

kulakshay commented

Thanks @Axbon!
But I am little confused here, do you mean that I use istanbuljs instead of istanbul-lib-instrument ?
Should I just add dependency for istanbuljs in my copy of istanbul-instrumenter-loader and remove istanbul-lib-instrument ?

  • ๐Ÿ˜•1
Axbon commented

That is a good question, I suppose the guys behind istanbuljs have to re-publish to npm so that the package comes from their "monorepo" since it was migrated there. There appears to be no "istanbuljs" on npm where you would get access to all packages there :( I'll try to find out

kulakshay commented

Thanks @Axbon, my current status is I can generate report in 'post' mode, but 'pre' mode is still not working.
So there is huge gap between previous coverage results (using isparta in pre mode) and current implementation.

unimonkiez commented

My coverage dropped about 10% while upgrading from isparta-loader (pre mode) to istanbul-instrumenter-loader (post mode).

Unable to change to pre without though..

EDIT:
Removed the 'enforce' on 'istanbul-instrumenter-loader' and moved it to be first in the loaders array (otherwise it wouldn't work), that made my coverage increase a lot but still 2% below isparta-loader.
Checking a sample file and still shows I have a problem (2/10 coverage produces 16.67%)

EDIT 2:
Managed to get it to work, all I needed to do is to upgrade to babel-loader v7.0.0 (which was released just several days ago), but still leave the loader to be first in the array.

You can check my repo with the implementation (webpack line).

documentation should be updated to state the solution.

  • ๐Ÿ‘3
kulakshay commented

@unimonkiez I'll definitely give it a shot, thanks !!!

mattlewis92 commented

The recommended path to getting babel + istanbul instrumentation to work is to use the babel plugin to instrument your code, rather than this loader. Just remove this loader and add that babel plugin to your babel config when testing and everything should work great! ๐Ÿ˜„