-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix(config): remove ts-node register() call to allow custom configuration #3274
base: master
Are you sure you want to change the base?
Conversation
…tion. Docs fixed.
@@ -27,7 +27,7 @@ try { | |||
} catch (e) {} | |||
|
|||
try { | |||
require('ts-node').register() | |||
require('ts-node') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gather you are saying: we don't need to call register because all we have no options?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fact that Karma does this is highly bothersome: #3329
Use of ts-node
is not a bad idea, but it should be optional, with some way to enable it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trusktr thanks for the example thats exactly the problem I encountered too. You can only call register()
once in your whole project after that it is not possible to change the options
given to the register
call. The register
call in your kamra.conf.ts
is simply ignored cause karma
already called it here. The only way to inject some other configuration is via the tsconfig.json
which needs to be the top level tsconfig.json
file. This is not suitable for all projects.
This will be a breaking change cause, if we remove the register
call we need a karma.conf.js
that calls register
for us. Another solution could be to allow the injection of a tsconfig.json
via a command line arg.
For me I was able to find a tsconfig.json
configuration that fits karma
and the rest of the project. But the way to this solution was very cumbersome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the breaking change info
https://www.conventionalcommits.org/en/v1.0.0/
and the doc change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be great to merge, but then the docs also need an update on how to add TS support when that is wanted.
The only easy workaround that I can think of is forking karma to disable the |
In my use case, because Karma and it's configuration are located inside |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(sorry for the duplicate)
Please add the breaking change info
https://www.conventionalcommits.org/en/v1.0.0/
and the doc change.
Can we please have this merged. It is blocking a PR in Angular builders that allows custom webpack configuration in Angular. |
It seems the problem was fixed via 474f4e1. Now it should be possible to define a JS configuration that can call ts-node's register function without interfering with the karma register call. I am currently unable to confirm if this solved the described problem cause I am not working anymore on any karma dependent project. |
1024b08
to
a985259
Compare
6fe6df0
to
56cda53
Compare
Calling
require('ts-node').register()
to determine iftypescript
is available prevents the custom configuration of theregister
call described in the documentation. I removed the unnecessaryregister
call and fixed the missingmodule.exports
assignment in the documentation.