-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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 ts.transpileDeclaration having empty sourceMapText when declarationMap is true #59314
Fix ts.transpileDeclaration having empty sourceMapText when declarationMap is true #59314
Conversation
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've verified this fixes the issue for my use case, but I'm not sure how to write a test for this. I didn't find any existing tests that I could use as a reference.
cc @weswigham
@@ -468,7 +468,6 @@ export const commonOptionsWithBuild: CommandLineOption[] = [ | |||
affectsBuildInfo: true, | |||
showInSimplifiedHelpView: true, | |||
category: Diagnostics.Emit, | |||
transpileOptionValue: undefined, |
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 being set to undefined
caused declarationMap
to be set to undefined
in options
TypeScript/src/services/transpile.ts
Lines 130 to 137 in f37482c
for (const option of transpileOptionValueCompilerOptions) { | |
// Do not set redundant config options if `verbatimModuleSyntax` was supplied. | |
if (options.verbatimModuleSyntax && optionsRedundantWithVerbatimModuleSyntax.has(option.name)) { | |
continue; | |
} | |
options[option.name] = option.transpileOptionValue; | |
} |
@@ -205,7 +205,7 @@ function transpileWorker(input: string, transpileOptions: TranspileOptions, decl | |||
addRange(/*to*/ diagnostics, /*from*/ program.getOptionsDiagnostics()); | |||
} | |||
// Emit | |||
const result = program.emit(/*targetSourceFile*/ undefined, /*writeFile*/ undefined, /*cancellationToken*/ undefined, /*emitOnlyDtsFiles*/ declaration, transpileOptions.transformers, /*forceDtsEmit*/ declaration); | |||
const result = program.emit(/*targetSourceFile*/ undefined, /*writeFile*/ undefined, /*cancellationToken*/ undefined, /*emitOnly*/ undefined, transpileOptions.transformers, /*forceDtsEmit*/ undefined); |
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.
Before we were passing true
to forceDtsEmit
, which caused this to evaluate to false:
TypeScript/src/compiler/emitter.ts
Line 907 in f37482c
sourceMap: !forceDtsEmit && compilerOptions.declarationMap, |
though I'm not sure if this is actually the correct fix. Perhaps it would be to just remove
!forceDtsEmit &&
from emitter.ts
.
Changing emitOnlyDtsFiles
to undefined
actually doesn't seem to matter, at least with the configuration I tested with, which seems kind of surprising.
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.
Well, no permutation of what I said results in all the tests passing, so clearly not as easy of a fix as I hoped.
Closing in favor of #59337 |
Fixes #59313