fix: Add ts-ignore to reassignment of generated function

[coreyfarrell]

Fixes istanbuljs/nyc#1292

[jedateach]

What's the required process to see this merged?

[jedateach]

Any idea if you can install git subdirectories as npm packages?
I'm attempting to test with npm i --save-dev github:istanbuljs/istanbuljs#ts-ignore-func-reassign, but that fails.

I might need to npm link it

[jedateach]

Ok, I can confirm I've tested this branch (using npm link) against my test repo, and no longer get build issues.

Will also test with my real repo.

[jedateach]

I'm now seeing this issue:

instrumented/Text.ts:5852:3 - error TS2376: A 'super' call must be the first statement in the constructor when a class contains initialized properties, parameter properties, or private identifiers.

5852   constructor(props: ConstructObj = (cov_25gnjz1tfh().b[0][0]++, null)) {
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
5853     cov_25gnjz1tfh().f[0]++;
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 ... 
5869     this.loadFonts();
     ~~~~~~~~~~~~~~~~~~~~~
5870   }

When instrumenting this code:

  constructor(props: ConstructObj = null) {
    super();
    if (props) {
      this.original = props;
      this.set(props);
    }
    this.loadFonts();
  }

[coreyfarrell]

@jedateach I should be able to merge / release this soon.

As for TS2376 there is currently no fix, see microsoft/TypeScript#8277 and microsoft/TypeScript#29374. Take the following example:

class A {
  constructor(public p: string) {
    super();
    this.otherProp = '1';
  }
}

Instrumented:

class A {
  constructor(public p: string) {
    cov_1zjww7ja73.f[0]++;
    cov_1zjww7ja73.s[0]++;
    super();
    cov_1zjww7ja73.s[1]++;
    this.otherProp = '1';
  }
}

We must increment the f[0] and s[0] counters before calling super(), TS2376 makes this 'illegal'. Any fix for this specific issue must come from TypeScript.

On your own project you might be able to work around this by running tsc on original TS source code to check typings, then run nyc --require=ts-node/register/transpile-only to test with coverage (test against original TS source code).

[jedateach]

Thanks for the tip - I'll give it a go.

Would be really great to have an established pattern for Typescript projects to be checking coverage.

I'm surprised Microsoft hasn't dedicated some effort to coverage, either by helping this project or doing their own. ...Unless I've missed something. I'm new to TS.

[jedateach]

In my case, I'm running the tests in Testem, not node. This means I need to instrument in an isolated step. Is it possible to achieve the same as nyc --require=ts-node/register/transpile-only tsc src/index.ts, but with separated instrument step?

I've been thinking it'd be a matter of getting tsc to ignore errors, but can't see how to do that.

[jedateach]

Looks like the Typescript team (2013) just test coverage using the produced JS. https://stackoverflow.com/a/17028570/918605
I might resort to this for now.

[coreyfarrell]

I'm pretty sure you can use nyc instrument --require=ts-node/register/transpile-only.

Testing coverage of the generated JS works but tsc injects branches / other code which often cannot be covered, this can be a pain if you are trying to hit 100% coverage.

[coreyfarrell]

istanbul-lib-instrument@4.0.2 is released to npm on the next tag. It'll be promoted to latest once I get status back from a couple tests I requested (probably the next couple days).

[jedateach]

I'm pretty sure you can use nyc instrument --require=ts-node/register/transpile-only.

I've now attempted this, and even though the required modules appear be loaded, the output produced is always typescript. I'm not sure how to get nyc instrument to also transpile.

nyc instrument --require=ts-node/register produces the same output, nor with any transpile errors.

[coreyfarrell]

I can take a look if you post a minimal demo repository. Keep in mind I'm not a TS developer so be sure to include whatever is required for ts-node/register to work. tsconfig.json I think?