@sheetalkamat @sandersn any thoughts on this one?

I'm not familiar with these functions.
@cubenoy22 Are you saying that both ts and SourceFile have members named (1) getLineAndCharacterOfPosition (2) getPositionOfLineAndCharacter?

If I'm reading your proposal correctly, I think you want to remove the properties on ts because they error when called. Is that correct?

I dont understand the suggestion here at all..
getPositionOfLineAndCharacter on ts handles if there is getPositionOfLineAndCharacter on sourceFileLike or not so why should that be issue.

@sandersn

Are you saying that both ts and SourceFile have members named (1) getLineAndCharacterOfPosition (2) getPositionOfLineAndCharacter?

Exactly.

@sandersn @sheetalkamat

Let me explain about two functions.

SourceFile is declared here:

These methods are easy to use (just call them).

And There are also two exported functions in scanner.ts:

Both functions take sourceFile: SourceFileLike parameter at first.
Of course, It is OK to pass SourceFile instances despite of SourceFile has own methods.
But I thought SourceFileLike is constructable like PromiseLike and ArrayLike:

  ts.getPositionOfLineAndCharacter({
    getLineAndCharacterOfPosition: () => /* ... */,
  }, line, number);

And I found a construction case in this project:

My above code is buildable despite of lacking text: string on any projects using TypeScript because SourceFileLike as public is here:

But as I wrote in description, Two functions (not methods of SourceFile) use /** internal */ marked members of SourceFileLike (I explain on below).

Should we expose only text: string on SourceFileLike instead of marking /** @internal */ to two functions?

getLineStarts function which is called in getLineAndCharacterOfPosition and getPositionOfLineAndCharacter mutates lineMap of SourceFileLike to cache. So lineMap?: readonly number[]; should be exposed too I think.

Thank you for explaination.. now i see what you mean.. The actual definition of SourceFileLike in compiler is internal but not in services so public one doesnt have the right definition. Got it

I think we should make SourceFileLike in compiler\types.ts not internal but make probably make lineMap internal.

Thank you for your reply. Let me write my thoughts. SourceFileLike in compiler\types.ts is here:

• Removing /** @internal */ at L.3460 exposes text and lineMap properties
  • L.3466 lineMap?: readonly number[]; is wrong definition (Built-in functions mutate this property if not exists), so applying /** @internal */ looks good for me
    • Adding lines field by the library is not expected behavior for users, so should we change its name to _lines and so on?
• This changes public APIs so "acknowledge required" before we make a pull request

Impact to internal codes

• Exposing and marking /** @internal */ cause no changes to codes
  • We need to change baseline
• If we rename lines to _lines, It must be fully cared

Impact to external codes

• I searched SourceFileLike in typescript.d.ts and I found another function using SourceFileLike here
  , so only three functions expose SourceFileLike to public now.
• SourceFileLike is not usable alone because of lacking text property (No one use these functions or pass SourceFile instances, I think)
• text property is a member of SourceFile:

  • TypeScript/src/compiler/types.ts

    Lines 3456 to 3565 in 3d24b85

    	export interface SourceFile extends Declaration {
    	readonly kind: SyntaxKind.SourceFile;
    	readonly statements: NodeArray<Statement>;
    	readonly endOfFileToken: Token<SyntaxKind.EndOfFileToken>;
    	fileName: string;
    	/* @internal */ path: Path;
    	text: string;
    	/** Resolved path can be different from path property,
    	* when file is included through project reference is mapped to its output instead of source
    	* in that case resolvedPath = path to output file
    	* path = input file's path
    	*/
    	/* @internal */ resolvedPath: Path;
    	/** Original file name that can be different from fileName,
    	* when file is included through project reference is mapped to its output instead of source
    	* in that case originalFileName = name of input file
    	* fileName = output file's name
    	*/
    	/* @internal */ originalFileName: string;
    	/**
    	* If two source files are for the same version of the same package, one will redirect to the other.
    	* (See `createRedirectSourceFile` in program.ts.)
    	* The redirect will have this set. The redirected-to source file will be in `redirectTargetsMap`.
    	*/
    	/* @internal */ redirectInfo?: RedirectInfo;
    	amdDependencies: readonly AmdDependency[];
    	moduleName?: string;
    	referencedFiles: readonly FileReference[];
    	typeReferenceDirectives: readonly FileReference[];
    	libReferenceDirectives: readonly FileReference[];
    	languageVariant: LanguageVariant;
    	isDeclarationFile: boolean;
    	// this map is used by transpiler to supply alternative names for dependencies (i.e. in case of bundling)
    	/* @internal */
    	renamedDependencies?: ReadonlyESMap<string, string>;
    	/**
    	* lib.d.ts should have a reference comment like
    	*
    	* /// <reference no-default-lib="true"/>
    	*
    	* If any other file has this comment, it signals not to include lib.d.ts
    	* because this containing file is intended to act as a default library.
    	*/
    	hasNoDefaultLib: boolean;
    	languageVersion: ScriptTarget;
    	/* @internal */ scriptKind: ScriptKind;
    	/**
    	* The first "most obvious" node that makes a file an external module.
    	* This is intended to be the first top-level import/export,
    	* but could be arbitrarily nested (e.g. `import.meta`).
    	*/
    	/* @internal */ externalModuleIndicator?: Node;
    	// The first node that causes this file to be a CommonJS module
    	/* @internal */ commonJsModuleIndicator?: Node;
    	// JS identifier-declarations that are intended to merge with globals
    	/* @internal */ jsGlobalAugmentations?: SymbolTable;
    	/* @internal */ identifiers: ESMap<string, string>; // Map from a string to an interned string
    	/* @internal */ nodeCount: number;
    	/* @internal */ identifierCount: number;
    	/* @internal */ symbolCount: number;
    	// File-level diagnostics reported by the parser (includes diagnostics about /// references
    	// as well as code diagnostics).
    	/* @internal */ parseDiagnostics: DiagnosticWithLocation[];
    	// File-level diagnostics reported by the binder.
    	/* @internal */ bindDiagnostics: DiagnosticWithLocation[];
    	/* @internal */ bindSuggestionDiagnostics?: DiagnosticWithLocation[];
    	// File-level JSDoc diagnostics reported by the JSDoc parser
    	/* @internal */ jsDocDiagnostics?: DiagnosticWithLocation[];
    	// Stores additional file-level diagnostics reported by the program
    	/* @internal */ additionalSyntacticDiagnostics?: readonly DiagnosticWithLocation[];
    	// Stores a line map for the file.
    	// This field should never be used directly to obtain line map, use getLineMap function instead.
    	/* @internal */ lineMap: readonly number[];
    	/* @internal */ classifiableNames?: ReadonlySet<__String>;
    	// Comments containing @ts-* directives, in order.
    	/* @internal */ commentDirectives?: CommentDirective[];
    	// Stores a mapping 'external module reference text' -> 'resolved file name' | undefined
    	// It is used to resolve module names in the checker.
    	// Content of this field should never be used directly - use getResolvedModuleFileName/setResolvedModuleFileName functions instead
    	/* @internal */ resolvedModules?: ESMap<string, ResolvedModuleFull | undefined>;
    	/* @internal */ resolvedTypeReferenceDirectiveNames: ESMap<string, ResolvedTypeReferenceDirective | undefined>;
    	/* @internal */ imports: readonly StringLiteralLike[];
    	// Identifier only if `declare global`
    	/* @internal */ moduleAugmentations: readonly (StringLiteral | Identifier)[];
    	/* @internal */ patternAmbientModules?: PatternAmbientModule[];
    	/* @internal */ ambientModuleNames: readonly string[];
    	/* @internal */ checkJsDirective?: CheckJsDirective;
    	/* @internal */ version: string;
    	/* @internal */ pragmas: ReadonlyPragmaMap;
    	/* @internal */ localJsxNamespace?: __String;
    	/* @internal */ localJsxFragmentNamespace?: __String;
    	/* @internal */ localJsxFactory?: EntityName;
    	/* @internal */ localJsxFragmentFactory?: EntityName;
    	/* @internal */ exportedModulesFromDeclarationEmit?: ExportedModulesFromDeclarationEmit;
    	/* @internal */ endFlowNode?: FlowNode;
    	}

  • So, exposing text: string causes no impact to any codes passing SourceFile instances

And I'll try this fix. if acknowledged to change public API.