Skip to content

Factor out repeated code and call one from another - #125

Merged
dragomirtitian merged 2 commits into
bloomberg:isolated-declarationsfrom
h-joo:refactorSourceMaps
Jan 8, 2024
Merged

Factor out repeated code and call one from another#125
dragomirtitian merged 2 commits into
bloomberg:isolated-declarationsfrom
h-joo:refactorSourceMaps

Conversation

@h-joo

Copy link
Copy Markdown

This is totally a non-functional change. No code modification, just moving around.

Comment threadsrc/compiler/emitter.ts Outdated
}

/** @internal */
export function getSourceMapDirectory(host: EmitHost, mapOptions: SourceMapOptions, filePath: string, sourceFile: SourceFile | undefined) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently trying to get rid of the emit host as much as possible since that has a lot of functionality that is not available in the DTE. Could we maybe find a way to subset the methods of EmitHost that actually used here? In #124 I manage to get rid of most of the unimplemented parts of emitHost and just keep the core functionality used in source maps:

constemitHost={getCurrentDirectory: ()=>transpileOptions.currentDirectory??".",getCanonicalFileName: createGetCanonicalFileName(!!compilerOptions.useCaseSensitiveFileNames),useCaseSensitiveFileNames: ()=>!!compilerOptions.useCaseSensitiveFileNames,getCompilerOptions: ()=>compilerOptions.compilerOptions,getCommonSourceDirectory: ()=>ensureTrailingDirectorySeparator(transpileOptions.commonSourceDirectory??"."),};

Maybe we can do something similar? I seem to remember I used Pick in some place to pick out just the actually used methods. Not sure if Pick is the best option, but maybe another type with just our methods of interest (or if we find an existing one taht does not grow the API surface too much that would also be great)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this? I made EmitHost -> CoreEmitHost

@h-joo
h-joo changed the base branch from isolated-declarations to isolated-declarations-safety-improvementsNovember 30, 2023 10:34
@h-joo

Copy link
Copy Markdown
Author

Should be merged after #124 is merged in.

@dragomirtitian
dragomirtitianforce-pushed the isolated-declarations-safety-improvements branch 3 times, most recently from 1ea970e to f9aa1b6CompareDecember 5, 2023 14:00
@h-joo
h-jooforce-pushed the refactorSourceMaps branch from 66783a3 to 7362d4bCompareDecember 6, 2023 12:51
@dragomirtitian
dragomirtitianforce-pushed the isolated-declarations-safety-improvements branch 2 times, most recently from b7d3c82 to 211b056CompareDecember 11, 2023 17:59
@dragomirtitian
dragomirtitianforce-pushed the isolated-declarations-safety-improvements branch 2 times, most recently from a2fab36 to c45ffafCompareJanuary 2, 2024 15:37
@h-joo
h-jooforce-pushed the refactorSourceMaps branch from 7362d4b to 2bc4a1dCompareJanuary 8, 2024 15:37
@h-joo
h-joo changed the base branch from isolated-declarations-safety-improvements to isolated-declarationsJanuary 8, 2024 15:38
Also make the functions use the new type CoreEmitHost
to take into account of methods that are not available
from DTE mode.
Signed-off-by: Hana Joo <hanajoo@google.com>
Signed-off-by: Hana Joo <hanajoo@google.com>
@h-joo
h-jooforce-pushed the refactorSourceMaps branch from 2bc4a1d to a97dfbcCompareJanuary 8, 2024 15:41
@dragomirtitian
dragomirtitian merged commit 04097f6 into bloomberg:isolated-declarationsJan 8, 2024
@h-joo
h-joo deleted the refactorSourceMaps branch January 8, 2024 15:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@h-joo@dragomirtitian