-
-
Notifications
You must be signed in to change notification settings - Fork 431
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
buildSolutionReferences with instance transformers #1274
buildSolutionReferences with instance transformers #1274
Conversation
Heads up, we've just renamed from This will have updated this PR but not your local branch. To apply there then:
|
merge transformers in case additional transformers were added somewhere
(because unit tests do not pass and it is no longer needed for transformers)
@johnnyreilly |
Thanks - I'll try and take a look this weekend |
Sorry - I forgot about this. Just updated the branch. Will take a look. |
Is |
Looking good I think. Two things:
|
customTransformers?: ts.CustomTransformers | ||
): ts.EmitResult { | ||
/* Invoke TS emit */ | ||
const result: ts.EmitResult = originalEmit( |
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 will have some bad effects .. emit is used for emitting dts files to determine shape of the file as well and take internal parameters and that signature is internal to typescript .. also I don’t think you want transformers for that reason ..
I have been thinking about your issue and was wondering if build could take getTransformers as function that returns transformers based on project path.. haven’t had chance to investigate that part yet but this approach is definitely not correct
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.
Oh thanks for that @sheetalkamat - very helpful!
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.
Yes, if we can pass customTransformers to build in SolutionBuilder, this solves the problem.
the done function on invalidatedProject already has the customTransformers parameter.
A function by projet path is ok, anyway in ts-loader there is only one global configuration so we would always pass the same transformers. A good solution would be to do like ttypescript (https://github.com/cevek/ttypescript) and read the configuration from tsconfig.json
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.
microsoft/TypeScript#43984 add the typescript side of the API
Should we close this now? |
Yes, I close it, I didn't know Typescript 4.3.1 was out... I will create another issue / PR to solve this using SolutionBuilder. |
Fixes #1025
I first tried a solution inspired from https://stackoverflow.com/questions/62026189/typescript-custom-transformers-with-ts-createwatchprogram/62132983#62132983
Where I used getNextInvalidatedProject from compiler api that is defined here :
https://github.com/microsoft/TypeScript/blob/master/src/compiler/tsbuildPublic.ts
This worked for standalone builds but was not working in watch mode.
In watch mode, it is SolutionBuilder that handles the emit and today there is no way to pass customTransformers to SolutionBuilder.
The final solution overrides the emit function and is based on https://github.com/nonara/ts-patch