Change __TS_CONFIG__ from a global variable to a jest configuration

[GeeWee]

• Issue
  It seems silly TS_CONFIG should be a global variable, as the test code does not and should not care about it.

I suggest putting it into a jest configuration key instead
e.g.

{
"jest" : {
"ts-jest": {
  "__TS_CONFIG__" : {
   
  },
}

This makes more sense, and will later streamline the process of adding more configurations as discussed in #213

This is however a breaking change, and will need deprecation warnings. I also think it would be prudent to support both ways of doing it for a few versions to make sure everyone is caught up.
It will be very easy for the end users to migrate.

[kulshekhar]

Sounds like a good idea. Although it might be better to use something other than ts-loader as it could lead to some confusion

[GeeWee]

Yeah It should definitely be ts-jest - I always confuse these two in my head. Edited the OP.

[GeeWee]

Do you want me to take a crack at this @kulshekhar ?

[kulshekhar]

@GeeWee sure!

The change itself should be straightforward but all the tests that use this would need to be updated as well. And the readme.

Also, for the time being, it might be a good idea to let both global > __TS_CONFIG__ and global > ts-jest > __TS_CONFIG__ work to avoid a breaking change.

We can probably show some sort of a warning for the users of the old setup so that by the time the next version of Jest (or the one after that) is out, we can complete the switch.

[GeeWee]

I'm thinking since we'll want to maintain both behaviours at the start, I'll duplicate all the tests for the new functionality, but just testing for the new key, and then maintain all the old tests unchanged, except that they'll now check for a warning message, to make sure the users are informed.

What is global in your example? Agree on both keeping the old behaviour for a little while and showing a warning.

[kulshekhar]

I think it might be enough to change the existing tests to use globals > ts-jest > __TS_CONFIG__ instead of duplicating them.

I used global (should've been globals) to indicate the global variable available from jest config - the current parent __TS_CONFIG__.

[GeeWee]

When you say global > ts-jest > __TS_CONFIG__ do you mean that you think global should take precedence?

[kulshekhar]

no .. I meant it like breadcrumbs. So global > ts-jest > __TS_CONFIG__ would mean ts-jest is an object in globals and __TS_CONFIG__ is within ts-jest, like so:

{
  "globals": {
    "ts-jest": {
      "__TS_CONFIG__": { }
    }
  }
}

[GeeWee]

Ah, I see.
I don't agree with putting it in "globals" though - I'd like to put them outside globals.
We have access to the entire jest configuration object, I was thinking something like this.

"jest": {
    "transform": {
      ".(ts|tsx)": "<rootDir>/node_modules/ts-jest/preprocessor.js"
    },
    ...
    "ts-jest": {
     "__TS_CONFIG__" : {}
  }
}

aka outside of the globals key.

[kulshekhar]

I think jest will show a warning on the console if it's done that way

[GeeWee]

Damnit, you're right. Globals it is then!

[bcruddy]

If this is implemented ts-jest should check both globals.ts-jest.__TS_CONFIG__ and globals.__TS_CONFIG__ and show a deprecation warning if __TS_CONFIG__ is found in globals until v21 to avoid breaking anyone. Before v21, which one takes will precedence?

[kulshekhar]

Yes, that'll probably be the best way. I think globals.ts-jest.__TS_CONFIG__ should take precedence.

[GeeWee]

Completely agree.

[kulshekhar]

@GeeWee are you working on this?

[GeeWee]

Not currently, focusing on Babel atm. Might get around to this, but it won't be for a week+

[bcruddy]

I can't actually contribute any code until next weekend due to legal stuff at work (IBM requires a bunch of legal gymnastics to contribute to open source but next Friday is my last day). I'd be more than happy to look into this if no one else has gotten to it by then.

[clayne11]

I have an issue with the way this is currently implemented. Trying to run jest with multiple projects (aka jest --projects packages/my-package-1 pacakges/my-package-2) results in the tsconfig.json file having the path starting at the root package rather than relative to the current project.

[GeeWee]

Interesting. Where is the jest --projects switch documented? It's not here

[clayne11]

It's mentioned in this blog post and also here in the docs:

https://facebook.github.io/jest/docs/en/configuration.html#projects-array-string.

Interestingly, the package.json config doesn't actually seem to work properly. The command line switch does work, though.

I think the the __TS_CONFIG__ var needs to have <rootDir> used within its config to make this work.

[GeeWee]

Interesting. This is definitely something we should fix, but it doesn't seem like it's close enough to this issue to be discussed here. I'd love it if you started a new issue with a repo that reproduces the error.

[morajabi]

I'm fixing this but should I use this syntax as it is now on README?

//This will skip babel transpilation
{
  "jest": {
    "globals": {
      "ts-jest": {
        "__TS_CONFIG__": "...."
      }
    }
  }
}

or simply skip globals?

[morajabi]

Sorry, What should I do with transpileIfTypescript function? It is called from src/default-retrieve-file-handler.ts and It has no access to config. How can I get config there to use instead of globals?

[kulshekhar]

@morajabi Thanks for picking this up :)

or simply skip globals?

We can't skip globals. Any non-standard jest configuration has to go in globals. The structure you've used looks fine. Although I'd like to split __TS_CONFIG into two distinct keys - tsconfigFile and tsConfig to make it clear. (this would require more changes, though) @GeeWee thoughts?

Sorry, What should I do with transpileIfTypescript function

There are two places that'll need change to address this - util.ts and transpile-if-ts.ts

What I'd suggest is

• create a function that retrieves the config setting
• place a call to this function in the above two places

This function should:

• currently, work with the present config format but display a warning on the console
• work with the new config format

If you have any more questions at any stage feel free to post them here!

[morajabi]

@kulshekhar Thanks kind man.

Although I'd like to split __TS_CONFIG into two distinct keys - tsConfigFile and tsConfig to make it clear. (this would require more changes, though)

What is the difference between them? Is tsConfigFile a config path and tsConfig raw config to override some pieces of the config file and thus they need to be merged?

[kulshekhar]

Is tsConfigFile a config path and tsConfig raw config to override some pieces of the config file and thus they need to be merged

Yes, to what they'd stand for. I should point out that, currently, the config is not merged. It is overwritten.

If both tsconfig and tsconfigFile are present, we should use only one and document which gets preference.

Edit: The more I think about this, the more I feel we shouldn't allow setting the raw config at all. It would be much simpler (for us and for the users) to just use a different tsconfig file (tsconfig.test.json, for instance) to tweak the test configuration

[morajabi]

Why not merge?

[kulshekhar]

Merging different configurations can lead to hard debug issues. Moreover, if someone wants to do this, tsconfig already has extends.

[GeeWee]

I think having two configuration entries is a pretty bad idea, as you'll need to remember what takes precedence, etc.
I'm okay with either the current approach (e.g. a string or a tsconfig object) or only having a .tsconfig file option, i.e. not allowing it to be passed inline.

[GeeWee]

Welcome aboard btw @morajabi

[kulshekhar]

I'm okay with either the current approach (e.g. a string or a tsconfig object) or only having a .tsconfig file option, i.e. not allowing it to be passed inline.

I'm strongly in favor of having only one, optional, tsconfigFile option.

[GeeWee]

I can't come up with a case where you'd need to do it inline, so this is fine with me.