Remove diagnostics, reflection support, debug info generation

[kulshekhar]

Most of the new issues users are facing are due to these.

• Diagnostics

  • I've long maintained that testing is not the place to look for type errors. Testing is to test the functionality implemented. Moreover, Jest doesn't enable this scenario because the transformers only receive one file at a time. If there's a requirement to check the code in a file against the rest of the files, Jest needs to be provide the supporting structure for it.
  • I caved in and accepted a PR with this feature but regret the decision now - this isn't something that absolutely needs to be in ts-jest

• Reflection support

  • I see the need for reflection support but the current implementation doesn't seem to have fixed any open issue related to them. For this reason, I'd like the related parts to be removed from the main branch. It can still continue to be available in another branch (or a fork). Once it is in working condition, it can be merged in

• Debug info generation

  • most new issues state that the debug file is not being generated. If this doesn't have a simple fix, it should be removed and the issue template updated accordingly

The intent behind this is to trim the codebase to the parts that work and cut out the rest of it because all it does is make maintenance difficult.

[GeeWee]

On diagnostics and reflection support:

1. What do you propose for type-errors then, e.g. for CI? Running tsc --noEmit? I'm not sure this is a reasonable request of people, since they might have e.g. subprojects
   Either way we need to decide this reasonably quick - let's get it into the next round of breaking changes.
2. I don't think the code is particularly messy - e.g. it's reasonably well sectioned off. On the other hand I agree it hasn't actually fixed anything.
   I'd like to be able to publish the repo to npm? Should we publish the other branch under a different tag, or something like that?
3. Agree. I think we should open a new issue and investigate - I think the debug info can be valuable, but if it only works half the time, we should remove it.
   Anecdotally, I sometimes think ts-jest is hard to debug when developing, and I do actually use the debug infrastructure to aid me in that.

[kulshekhar]

1. Yes, something like that. And yes, the impending round of breaking changes was what triggered this post. (an opportunity to clean up things and remove parts that aren't working well). I've never been entirely comfortable with this going in ts-jest and given the drag on performance, I'm not sure that it helps more than harms.

2. I don't think it's messy at all - I didn't mean to give that impression. What I meant is that while it isn't solving any outstanding issues, it might be a good idea to have it in a branch and/or a fork.

I'd like to be able to publish the repo to npm? Should we publish the other branch under a different tag, or something like that?

What do you think about forking this and publishing under a new name (ts-jest-experimental maybe)? I'll add you to the owner list on npm as well so you should be able to publish it too.

3. Yeah, I definitely see the value in having this (I was thrilled when it was merged in) but if it doesn't always work, it creates more confusion. Also, I factored in that we both don't have a lot of time these days and hence stated that if it doesn't have a quick fix, only then it should be removed.

[GeeWee]

Hm, I'm not sure. It's an opt-in performance problem though, only. So does it really matter?

Yeah I'm okay with keeping it in a seperate fork. I think I saw a bot that published PR's on npm under namespaces at some point, e.g. ts-jest@pr-ts-language-server for any outstanding PR's. could that work too?

Let's keep it in for now I think. I'll spend some time investigating why it only works sporadically when I have the time.

[kulshekhar]

1. It's opt-in but we already have had a couple of issues related to performance for it. If it was something that would just sit in quietly and help users, it'd be fine. But it being supported makes users (rightly) expect good performance - I don't see how performance in that can be improved. Until it can, this can also remain in fork if you still want this in.

2. Okay then, this can be forked - I'm fine with any naming scheme but think putting it under a namespace might be challenging because that should also involve moving ts-jest under the namespace (which would be too disruptive). A separate package (ts-jest-experimental, ts-jest-extra, etc.) should do I think.

3. Fair enough - this isn't as urgent and can be removed later if needed

[GeeWee]

1. I think that's fair enough. The performance should be improved when we implement a proper language server API. (At least there's a comment in ts-node saying it's 3x faster)

2. Sorry - not a namespace. I meant a label, like @beta , @Alpha @Pre-release or something like that.

3. Agree.

[kulshekhar]

2. okay. We should probably maintain this in another branch instead of a fork then

how do you want to proceed?

[GeeWee]

2. Agree.

First one to rip the code out and put it in another branch gets a cookie?
I think we should just do all the breaking changes, and then wait with releasing until next jest release, assuming it is soon. I've tweeted to try and figure out their release schedule https://twitter.com/GeeWengel/status/998899189840056322

I'll try to figure out if we can publish branches automatically somehow.

I'm at https://www.npmjs.com/~geewee if you want to give me publish access (brand new account - aww yeah!)

How does that sound?

[cspotcode]

What's the (2) reflection / language server issue? Is the goal to spawn a single instance of the languageService, then allow all processes in Jest's worker pool to read from it?

[kulshekhar]

@GeeWee I've added you on npm. A couple of things (for security reasons)

• please enable 2FA on your npm account
• if it looks like you can't work on or intend to take a break from ts-jest for an extended period of time, please let me know so that I can temporarily revoke your account's access to ts-jest on npm

I'll try to figure out if we can publish branches automatically somehow.

I think this should work - on a branch, set the package version to something like actual-version-experimental, add and publish a tag to Github from that branch, npm publish from that branch.

[kulshekhar]

@cspotcode I think that's correct but I'm not 100% certain.

[GeeWee]

• 2FA is enabled.
• Agree.

Yeah optimally we'd allow all workers to read from the same languageServer. It seems like the languageServer transpiles a lot of different files for each file, and I'd hate to do a lot of IO for each file, for each process.

[alan-agius4]

I was using the Language Service for a couple of days, in a large project.

From what I learned is;

1. I found quite a couple of issues that otherwise I would have missed using the traditional module compiler.
2. Was way faster when using diagnostics over enableTsDiagnostics
3. Also, I am not sure about this, but potential this could have solved the const enum issue.

To say the truth, I am a bit disappointing that it was removed, as now my tests will take way longer considering I want to be able to use diagnostics, as this was one of the coolest and exciting features of ts-jest.

[GeeWee]

Yeah it potentially solves const enum. I think we'll add it back in, but we're not there quite yet I think.

[huafu]

First of all, I did read the whole thread rapidly, sorry if I missed something.
I just wanted to add here before someone tries to re-implement something that might have been worked on in beta branch. Here is what exists in beta branch now, published as @next and described in #697, related to what is in this thread:

• diagnostics: user can filter which code(s) are throwing or not, and from which file(s)
• type-checking (ie using language service) is working, and fast
• debug file is relative to cwd, and can even be defined with an absolute path (TS_JEST_LOG, defaulting to ts-jest.log when old var TS_JEST_DEBUG is set to true). It's much more complete, and in JSON format

const enum are working when typeCheck: true is set (and we are thinking about making typeCheck = true by default once we have enough feedback and/or could benchmark it more)

@cspotcode the new ts compiler part of ts-jest's code has been heavily inspired from ts-node. Here an ugly but quite understandable diagram of how ts-jest works in beta branch:

[kulshekhar]

Yes, with the beta (which will become the regular release the next major version), this issue isn't that relevant. It can be closed after the next major version is released

[huafu]

...and I'm actually realizing that in the compiler one part needs to be added: custom AST transformers are called. For now only the one to hoist jest.mock calls, but plan is to read some other files. It could be useful for modules using ts-jest to build jest presets such as react and especially angular. What's missing in beta for that is only the "read from config which files to use as custom transformers and use them!". Format for those files should actually be the same format as the on in src/transformers/ dir.

[cspotcode]

Do you want to use my hoisting implementation or do you have your own? One thing to keep in mind, jest's Babel hoister does 2 things: it hoists calls and it throws diagnostics when hoisted declarations access variables that they shouldn't. In TypeScript this is best handled by a language service plugin, so I started writing one.

…

On Sun, Sep 2, 2018, 10:15 AM Huafu Gandon ***@***.***> wrote: ...and I'm actually realizing that in the compiler one part needs to be added: custom AST transformers are called. For now only the one to hoist jest.mock calls, but plan is to read some other files. It could be useful for modules using ts-jest to build jest presets such as react and especially angular. What's missing in beta for that is only the "read from config which files to use as custom transformers and use them!". Format for those files should actually be the same format as the on in src/transformers/ dir. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#529 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAW-uP7rV42QOulY1hcdiPTwwMGwByChks5uW-f-gaJpZM4UGEv5> .

[huafu]

@cspotcode you should definitely come on slack so we can talk about this ;-)

Briefly: hoisting is done, I've tried your PR (quickly, or at least I think it was your PR) but couldn't make it work - anyway I saw it after I implemented mine :-/
Language service is done and we are actually talking about whether it should be the default instead of isolated modules.

But yeah, please come over slack so we can chat better about it ;-)