-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Add SolutionBuilderHostBase.getCustomTransformers to be used when emitting. #44489
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
Conversation
…tting. This allows not having to specify the transformers during normal watch scneario Builds on top of #43984
@DanielRosenwasser we would want to cherry pick this to release-4.3 as well |
@typescript-bot cherry-pick this to release-4.3 |
Heya @sheetalkamat, I've started to run the task to cherry-pick this into |
Hey @sheetalkamat, I couldn't open a PR with the cherry-pick. (You can check the log here). You may need to squash and pick this PR into release-4.3 manually. |
@@ -785,7 +786,7 @@ namespace ts { | |||
emit: (targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers) => { | |||
if (targetSourceFile || emitOnlyDtsFiles) { | |||
return withProgramOrUndefined( | |||
program => program.emit(targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers) | |||
program => program.emit(targetSourceFile, writeFile, cancellationToken, emitOnlyDtsFiles, customTransformers || state.host.getCustomTransformers?.(project)) |
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.
Without knowing anything about custom transformers or how they're actually used, this seems a little strange to me. If there's a way to pass them to emit, why doesn't the consumer do that, rather than embed them in the host? Why is it (potentially) called separately for each emit
call? Can they change over time? If not, should they be stored somewhere? And why ||
, rather than (e.g) ?.concat()
?
I'm pretty sure this change accomplishes what it needs to accomplish, but it feels a little bit hastily done (assuming any of my intuition about what it's supposed to do is correct). Do you have a better understanding, @andrewbranch?
This allows not having to specify the transformers during normal watch scneario
Builds on top of #43984
This is in response to #43984 (comment)