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

Request assistance with nyc to ts-node/register incompatibility #813

Closed
coreyfarrell opened this issue Apr 2, 2019 · 5 comments
Closed

Comments

@coreyfarrell
Copy link

I'm an nyc maintainer working on istanbuljs/nyc#1002. I already have a patch in review to address the issue with coverageData.hash = hash;. I have not found a solution for error TS2351: Cannot use 'new' with an expression whose type lacks a call or construct signature.. The instrumented code always contains:

var Function = function () {}.constructor;
var global = new Function("return this")();

Worth noting node -e 'console.log(Function === function() {}.constructor);' will print true, unfortunately it's possible (in JavaScript) for Function to be redefined. For example the following logs false to the console:

console.log(Function === function () {}.constructor);
function Function() {
  // I don't know why someone would name a function `Function` but it has happened.
}

I've tried the following:

var FunctionReplacement = function () {}.constructor;
var globalFinder = Function === FunctionReplacement ? Function : FunctionReplacement;
var global = new globalFinder("return this")();

The hope was that in a TypeScript project nobody would have replaced Function, so globalFinder would be set to Function. ts-node still rejected this code with the same error. Would it be possible for ts-node/register to understand that function () {}.constructor === Function? I'd like to make it easier to get code coverage for projects using ts-node/register, any suggestion would be appreciated.

@blakeembrey
Copy link
Member

@coreyfarrell Hmm, I've always used ts-node to transpile before passing the results to Istanbul. Is my understanding here that you want the wrapper code of Istanbul to be valid TypeScript correct? Can you detect if you're in TypeScript? Not that you probably want to do so, but I'm looking at the TypeScript types and they're pretty non-helpful for this use-case. Since the Function type used for the constructor is not the same as the FunctionConstructor type declare as the global Function.

The workarounds I see are:

  1. Improve TypeScript types for function () {}.constructor properties
  2. Find a way to write this with good inferred types
  3. Remove usage of .constructor since it returns the wrong type and rely on Function directly

I'll keep thinking a little on 2, but nothing comes to mind right now as a workaround with annotating it explicitly with TypeScript types.

@coreyfarrell
Copy link
Author

I want the wrapper code to be tolerated as TypeScript, it does need to be valid JavaScript though (this is first priority). I think the idea behind instrumenting the TypeScript before transpiling it is to avoid generated branches. I'm unsure what you mean about Function vs FunctionConstructor since function () {}.constructor is the same exact value as the global Function. Is this a situation where the same value has a different type depending on where you get it from? Would it be possible to unify the types since they are the same thing?

I'm not sure about detecting that we're in TypeScript. Maybe we could omit the var Function = function () {}.constructor; if the source has a ts extension. I'm not sure how else we would detect this?

@blakeembrey
Copy link
Member

Is this a situation where the same value has a different type depending on where you get it from?

Yes.

Would it be possible to unify the types since they are the same thing?

Probably, that would be point 1. You'd need to submit an issue to TypeScript though.

@blakeembrey
Copy link
Member

blakeembrey commented Apr 2, 2019

Related to microsoft/TypeScript#3841.

Edit: It may be enough for now to submit a PR changing https://github.com/Microsoft/TypeScript/blob/3f3444be80dc939eb20db52fa98a206dba31e077/src/lib/es5.d.ts#L252 to add constructor: FunctionConstructor.

@coreyfarrell
Copy link
Author

I found the best immediate solution. We inject the coverage from a babel callback visitor.Program.exit, I added a check for path.scope.getBinding('Function'). This way we can output var Function = function () {}.constructor; only if the global Function was replaced, otherwise we use the built in. If someone replaces Function in TypeScript that's probably an error anyways.

Thanks for helping me understand this and linking me to the TypeScript issue.

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