Future of ts-jest babel integration

[GeeWee]

Continuation of discussion in #221

This is where we look at how we want to handle babel in the future, default modules and hoisting mock statements.

[GeeWee]

Responding to comment:

What are 'edge cases' to us maintaining ts-jest is really the 'main case' for users encountering them. No one but these users are best placed to address these issues. What we should focus on, imo, is making it straightforward for users to tune ts-jest to suit their needs.
With that in mind, here's how I think ts-jest needs to evolve:
remove all special handling of synthetic defaults (or anything else). Just process the source code according to the tsconfig settings
create a hook that allows users to pass the output of tsc through any function they provide. The default version of this hook can use babel to address #227

---

Alright, so it seems like we both agree on one thing:

The default case, no matter how it's handled should per default

• compile es6->commonJS modules if allowSyntheticDefaults is set to true
• Hoist the jest.mock calls to the top

The two suggestions currently proposed are:

• Have this be the default functionality. Use the skipBabel flag to opt out. If users want more customizations they'll have to write their own damn transformer.
• Have a hook function that the users can overwrite somehow (point to it via globals?) Use a default hook if no custom hook is provided.

What do we see as the pros and cons of each, and are there other ways?

[kulshekhar]

I'd put it like this

• transpile typescript. No special handling for allowSyntheticDefaultImports or anything else. Just let tsc compile based on the tsconfig that the user has specified. This allows the user to get ts-jest to do exactly what they want.
• hoist the jest.mock calls to the top

skipBabel won't be needed after this, right?

The hook can be invoked in a manner similar to how jest invokes its transformers. Users can specify a file that exports a function with a particular signature. If this is present, ts-jest will call it.

The question here is - how would this hook affect the use of babel for hoisting?

• override the original babel call with the hook. This will make the hook responsible for hoisting
• invoke the hook after hoisting. This will result in two calls to babel and be a bit slower
• make this behavior configurable using skipBabel. This will increase the complexity of the ts-jest API but avoids the pitfalls of the previous two options

Whatever we choose to do, we need to document it well!

[GeeWee]

Definitely agree on the documentation.

Another alternative: I imagine the only thing people will want to do is do some sort of babel transpilation.
What if we simply use an existing .babelrc if it exists, and if not, default to simply hoisting the mock calls and transpile the default modules if needed? This way if people need specific configurations they can simply provide their own .babelrc, and otherwise we default to sensible defaults without having to implement a hook api.

[kulshekhar]

What if we simply use an existing .babelrc if it exists, and if not, default to simply hoisting the mock calls and transpile the default modules if needed?

Wouldn't we be introducing babel related complexity in ts-jest by doing this? I personally don't know anything about babel and anything related that could morph into a maintenance issue scares me :)

The only reason we'd be using a default setup to hoist mock calls is because there isn't a typescript only alternative. I haven't really investigated that issue. If there's a way to do this without using babel, that'd be ideal.

It think it would be alright to use an existing .babelrc BUT only if we don't have to do anything more than invoke babel-jest with its.

otherwise we default to sensible defaults without having to implement a hook api

There are some concerns I have around this. How would we be able to decide what are 'sensible defaults'? What about users who need some sort of programmatic way of manipulating tsc's output in, for example, a customized pipeline. Something like this will come up - it always does 😄 The only long term solution seems (at least to me) to provide a way for users to hook into the process.

--

Basically, my preference would be to allow configuration for all things typescript and use a fixed setup for anything babel related. If this fixed setup doesn't work for someone, we can make it simple for them to create their own setup on top of ts-jest, using hooks.

That said, I can see the sense in allowing some sort of configurability for babel because some of the users might have to use babel out of necessity and not choice and they may not be familiar with babel.

What I propose is:

• focus our current efforts on typescript compilation and using babel for hoisting (with no configurability)
• structure the new ts-jest such that we can allow and support babel configuration in future, through a flag or some other mechanism. So, for example, what you've said about .babelrc can be taken care of. This can be done in small chunks, if required. If at any point there's enough knowledge and bandwidth with the maintainers of ts-jest to support babel related stuff, we can make it fully supported.

This would allow us, in the short term, to focus on getting the default ts-jest setup to work for cases that don't need babel related configuration. We won't spread ourselves too thin. Once we have that locked down, we can start work on making babel a first class option here.

[GeeWee]

I see your point about the complexity.
I think a sensible default is always to hoist, and I think a sensible default is to transpile modules if allowSyntheticDefault is set to true. I think these are the correct defaults for almost everyone.

I feel like if people need a customized pipeline they're welcome to create their own preprocessor that runs through ts-jest and then theirs afterwards. I'm a little wary if we allow people to add their own custom hooks, then helping them with that aspect is going to fall on our shoulders.

I like your way forward - I still feel like we should transpile the ES6 modules per default as well, however.
(Also we need to resolve how we'll handle the fact that babel currently wrecks our coverage (#211) - do you have any good ideas? I still think copying over the source code from babel-jest, modifying it a little and using that is our best bet.)

[kulshekhar]

transpile modules if allowSyntheticDefault is set to true

could you clarify what you mean by that.

I'm a little wary if we allow people to add their own custom hooks, then helping them with that aspect is going to fall on our shoulders

I actually think it's the other way around. If we allow configurability, supporting that becomes our job (sort of). If users write their own hook, it's clear that'll be their responsibility. As things mature, we could even provide sample hooks for common scenarios

Also we need to resolve how we'll handle the fact that babel currently wrecks our coverage

For the basic setups, coverage actually works well. We have tests that check that. I think once we minimize the exposed surface area, the linked issue will take care of itself.

I still think copying over the source code from babel-jest, modifying it a little and using that is our best bet.

What's the harm in importing and using babel jest for hoisting? In the worst case, if they change their API, we can pin down an old version. Till then, we can get all updates/fixes directly from there

[GeeWee]

Yes - sorry. I mean create the synthetic default imports - exactly as we do currently.

I actually think it's the other way around. If we allow configurability, supporting that becomes our job (sort of). If users write their own hook, it's clear that'll be their responsibility. As things mature, we could even provide sample hooks for common scenarios

I think we agree. We both want to minimize configurability, as to not support it - I think maybe we should leave this debate for later?

For the basic setups, coverage actually works well. We have tests that check that.c

Yes, except they currently break with allowsynthethicdefaultimports
What do you mean by For the basic setups, coverage actually works well. We have tests that check that. I think once we minimize the exposed surface area, the linked issue will take care of itself. ?

What's the harm in importing and using babel jest for hoisting? In the worst case, if they change their API, we can pin down an old version. Till then, we can get all updates/fixes directly from there

Normally I think it'd be an excellent idea - except that it breaks the coverage, and the only fix around it I see, is to modify a few line, or mock a function in babel-core.

What other alternatives do you see?

[kulshekhar]

I mean create the synthetic default imports - exactly as we do currently.

ok. What I meant was we stop handling synthetic defaults in a special manner.

We both want to minimize configurability, as to not support it - I think maybe we should leave this debate for later?

That makes sense. We can try to converge on something a bit later.

Yes, except they currently break with allowsynthethicdefaultimports. What do you mean by For the basic setups, coverage actually works well. We have tests that check that. I think once we minimize the exposed surface area, the linked issue will take care of itself. ?

I mean that the current code is too brittle. Specially handling parts like synthetic defaults and babel introduce complexity that can lead to subtle, hard to find bugs. Once me make the processing pipeline simpler (based on this discussion), bugs like these will fall into one of the following categories:

• simpler to debug and fix if the fix lies in ts-jest
• clear that the fix lies outside ts-jest (in jest, babel-jest or in something like the coverage processor)

the only fix around it I see, is to modify a few line, or mock a function in babel-core.

I didn't consider this when making my previous post. In this case, copying and modifying the bit we need makes sense. Although we should treat that code as part of ts-jest and not a fork that needs to be kept in sync. Is that feasible?

[GeeWee]

ok. What I meant was we stop handling synthetic defaults in a special manner.

I see, how do you want to handle the transition period between not having the hook architecture / whatever in place, and removing the special handling? This transpilation I think, is vital to a lot of peoples use cases - it definitely is to mine.

I mean that the current code is too brittle. Specially handling parts like synthetic defaults and babel introduce complexity that can lead to subtle, hard to find bugs. Once me make the processing pipeline simpler (based on this discussion), bugs like these will fall into one of the following categories:
simpler to debug and fix if the fix lies in ts-jest
clear that the fix lies outside ts-jest (in jest, babel-jest or in something like the coverage processor)

I don't necessarily agree the code is too brittle, but I do agree that a simpler processing pipeline would be nicer. However I think we need to consider a lot of the complexity comes from us using babelJest in a way it is not meant to be used.

I didn't consider this when making my previous post. In this case, copying and modifying the bit we need makes sense. Although we should treat that code as part of ts-jest and not a fork that needs to be kept in sync. Is that feasible?

Yeah I think that could work, but we'll introduce a lot of dependencies. I'm okay with this.

[kulshekhar]

I see, how do you want to handle the transition period between not having the hook architecture / whatever in place, and removing the special handling? This transpilation I think, is vital to a lot of peoples use cases - it definitely is to mine.

Whatever we choose to do, we should allow the current setup to work as well. For a few versions, we can keep the current pipeline the default so that it doesn't break anything. At the same time, we show a message telling users about the impending change. We'll have a flag for users to switch between the current pipeline and the new pipeline. After a few versions, we can make the new pipeline the default one.

Edit: Alternatively, we could add a new preprocessor file that the users can user if they want the new pipeline. So we avoid flags

I don't necessarily agree the code is too brittle, but I do agree that a simpler processing pipeline would be nicer. However I think we need to consider a lot of the complexity comes from us using babelJest in a way it is not meant to be used.

We agree there. It is the babel-jest related part that makes the code brittle - because of how it is currently done.

Yeah I think that could work, but we'll introduce a lot of dependencies. I'm okay with this.

as am I

[GeeWee]

Alright, so currently what we want to do moving forward in the short-term is to:

• Grab a copy of babel-jest into ts-jest, so we have full control, and are able to fix TypeError: mappedCoverage.addStatement is not a function #211
• Make the babel transpilation seperate from the preprocessor, but simply called after the tsc transpilation
• And skippable by SkipBabel
• Maintain the current default behaviour, but also hoist jest.mock calls.
• Then find out what the optimal migration plan is.

Does this seem like an okay short term strategy ?

[kulshekhar]

yup!

[GeeWee]

I will see if I can get around to it in a reasonable timeframe :)

[kulshekhar]

we can share the workload - I'm sure none of us have time to work on it full time :)

[kulshekhar]

https://github.com/kulshekhar/ts-jest/tree/new-pipeline ?

[GeeWee]

Yes - excellent!

[GeeWee]

So porting the babel pipeline was actually really easy. I've done it in the branch, PR at #236. I've started the foundation for a hook architecture of some sort, but as agreed on, it's a WIP.

This should allow us to hopefully start fixing some babel related issues.

[GeeWee]

I think I closed this by mistake. I have merged the #236 in.

[kulshekhar]

Do we still need this issue open? I don't think we have anything babel related to look at any more apart from the useBabelRC flag (that has its own open issue)

[GeeWee]

Agreed.