Skip to content
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

Resolve the modue names with "modulename.json" to json files when doing node module resolution #22167

Merged
merged 25 commits into from
May 4, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
5771adb
Resolve Json file when module resolution strategy is node
sheetalkamat Jan 17, 2017
ee2e267
Replace usage of jsonObject on JsonSourceFile
sheetalkamat Jan 26, 2018
3572fad
Bind and resolve the module for json file
sheetalkamat Jan 17, 2017
00b6c32
Fix emit for json file
sheetalkamat Jan 17, 2017
a1922fd
More tests and also do not add index signature for json module
sheetalkamat Jan 17, 2017
ca590d6
Need allowJs to be true to use the json module resolution
sheetalkamat Jan 26, 2018
a790a92
Parse all json values at root
sheetalkamat Jan 30, 2018
4257c2f
Resolve the .json file only if module name contains the extension
sheetalkamat Feb 23, 2018
23a7e2f
Report errors about correctness of the json file
sheetalkamat Feb 24, 2018
4f1640d
Verify the output file paths for the json module emit
sheetalkamat Feb 24, 2018
c154b81
Ensure our readonly emptyArray stays non modified.
sheetalkamat Feb 26, 2018
2071466
Merge branch 'master' into requireJson
sheetalkamat Mar 8, 2018
2d9af0b
Merge branch 'master' into requireJson
sheetalkamat Mar 10, 2018
28a988d
Merge branch 'master' into requireJson
sheetalkamat Mar 12, 2018
b4e0062
Merge branch 'master' into requireJson
sheetalkamat Mar 16, 2018
9f72415
Merge branch 'master' into requireJson
sheetalkamat Apr 5, 2018
a143963
Resolve json modules only when --resolveJsonModule is specified
sheetalkamat Apr 5, 2018
9d3ad54
Reverted unintentional formatting changes
sheetalkamat Apr 6, 2018
ce08af4
Merge branch 'master' into requireJson
sheetalkamat Apr 13, 2018
579748b
Merge branch 'master' into requireJson
sheetalkamat Apr 30, 2018
c7b2d92
PR feedback
sheetalkamat Apr 30, 2018
97df079
PR feedback
sheetalkamat May 1, 2018
15f9ea3
Merge branch 'master' into requireJson
sheetalkamat May 3, 2018
4e6586d
PR feedback
sheetalkamat May 4, 2018
c4143ae
Update the message for resolveJsonModule as per PR feedback
sheetalkamat May 4, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ namespace ts {
return InternalSymbolName.Index;
case SyntaxKind.ExportDeclaration:
return InternalSymbolName.ExportStar;
case SyntaxKind.SourceFile:
// json file should behave as
// module.exports = ...
return InternalSymbolName.ExportEquals;
case SyntaxKind.BinaryExpression:
if (getSpecialPropertyAssignmentKind(node as BinaryExpression) === SpecialPropertyAssignmentKind.ModuleExports) {
// module.exports = ...
Expand Down Expand Up @@ -2234,6 +2238,13 @@ namespace ts {
if (isExternalModule(file)) {
bindSourceFileAsExternalModule();
}
else if (isJsonSourceFile(file)) {
bindSourceFileAsExternalModule();
// Create symbol equivalent for the module.exports = {}
const originalSymbol = file.symbol;
declareSymbol(file.symbol.exports, file.symbol, file, SymbolFlags.Property, SymbolFlags.All);
file.symbol = originalSymbol;
}
}

function bindSourceFileAsExternalModule() {
Expand Down
8 changes: 6 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2207,7 +2207,7 @@ namespace ts {
}

// May be an untyped module. If so, ignore resolutionDiagnostic.
if (resolvedModule && !extensionIsTypeScript(resolvedModule.extension) && resolutionDiagnostic === undefined || resolutionDiagnostic === Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type) {
if (resolvedModule && !resolutionExtensionIsTypeScriptOrJson(resolvedModule.extension) && resolutionDiagnostic === undefined || resolutionDiagnostic === Diagnostics.Could_not_find_a_declaration_file_for_module_0_1_implicitly_has_an_any_type) {
if (isForAugmentation) {
const diag = Diagnostics.Invalid_module_name_in_augmentation_Module_0_resolves_to_an_untyped_module_at_1_which_cannot_be_augmented;
error(errorNode, diag, moduleReference, resolvedModule.resolvedFileName);
Expand Down Expand Up @@ -4718,6 +4718,10 @@ namespace ts {
return links.type = anyType;
}
// Handle export default expressions
if (isSourceFile(declaration)) {
const jsonSourceFile = cast(declaration, isJsonSourceFile);
return links.type = jsonSourceFile.statements.length ? checkExpression(jsonSourceFile.statements[0].expression) : emptyObjectType;
}
if (declaration.kind === SyntaxKind.ExportAssignment) {
return links.type = checkExpression((<ExportAssignment>declaration).expression);
}
Expand Down Expand Up @@ -15597,7 +15601,7 @@ namespace ts {
const contextualType = getApparentTypeOfContextualType(node);
const contextualTypeHasPattern = contextualType && contextualType.pattern &&
(contextualType.pattern.kind === SyntaxKind.ObjectBindingPattern || contextualType.pattern.kind === SyntaxKind.ObjectLiteralExpression);
const isInJSFile = isInJavaScriptFile(node);
const isInJSFile = isInJavaScriptFile(node) && !isInJsonFile(node);
const isJSObjectLiteral = !contextualType && isInJSFile;
let typeFlags: TypeFlags = 0;
let patternWithComputedProperties = false;
Expand Down
179 changes: 95 additions & 84 deletions src/compiler/commandLineParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,12 @@ namespace ts {
category: Diagnostics.Advanced_Options,
description: Diagnostics.Enable_tracing_of_the_name_resolution_process
},
{
name: "resolveJsonModule",
type: "boolean",
category: Diagnostics.Advanced_Options,
description: Diagnostics.Include_modules_imported_with_json_extension
},
{
name: "listFiles",
type: "boolean",
Expand Down Expand Up @@ -953,9 +959,9 @@ namespace ts {
* Read tsconfig.json file
* @param fileName The path to the config file
*/
export function readJsonConfigFile(fileName: string, readFile: (path: string) => string | undefined): JsonSourceFile {
export function readJsonConfigFile(fileName: string, readFile: (path: string) => string | undefined): TsConfigSourceFile {
const textOrDiagnostic = tryReadFile(fileName, readFile);
return isString(textOrDiagnostic) ? parseJsonText(fileName, textOrDiagnostic) : <JsonSourceFile>{ parseDiagnostics: [textOrDiagnostic] };
return isString(textOrDiagnostic) ? parseJsonText(fileName, textOrDiagnostic) : <TsConfigSourceFile>{ parseDiagnostics: [textOrDiagnostic] };
}

function tryReadFile(fileName: string, readFile: (path: string) => string | undefined): string | Diagnostic {
Expand All @@ -973,58 +979,62 @@ namespace ts {
return arrayToMap(options, option => option.name);
}

let _tsconfigRootOptions: Map<CommandLineOption>;
let _tsconfigRootOptions: TsConfigOnlyOption;
function getTsconfigRootOptionsMap() {
if (_tsconfigRootOptions === undefined) {
_tsconfigRootOptions = commandLineOptionsToMap([
{
name: "compilerOptions",
type: "object",
elementOptions: commandLineOptionsToMap(optionDeclarations),
extraKeyDiagnosticMessage: Diagnostics.Unknown_compiler_option_0
},
{
name: "typingOptions",
type: "object",
elementOptions: commandLineOptionsToMap(typeAcquisitionDeclarations),
extraKeyDiagnosticMessage: Diagnostics.Unknown_type_acquisition_option_0
},
{
name: "typeAcquisition",
type: "object",
elementOptions: commandLineOptionsToMap(typeAcquisitionDeclarations),
extraKeyDiagnosticMessage: Diagnostics.Unknown_type_acquisition_option_0
},
{
name: "extends",
type: "string"
},
{
name: "files",
type: "list",
element: {
name: "files",
_tsconfigRootOptions = {
name: undefined, // should never be needed since this is root
type: "object",
elementOptions: commandLineOptionsToMap([
{
name: "compilerOptions",
type: "object",
elementOptions: commandLineOptionsToMap(optionDeclarations),
extraKeyDiagnosticMessage: Diagnostics.Unknown_compiler_option_0
},
{
name: "typingOptions",
type: "object",
elementOptions: commandLineOptionsToMap(typeAcquisitionDeclarations),
extraKeyDiagnosticMessage: Diagnostics.Unknown_type_acquisition_option_0
},
{
name: "typeAcquisition",
type: "object",
elementOptions: commandLineOptionsToMap(typeAcquisitionDeclarations),
extraKeyDiagnosticMessage: Diagnostics.Unknown_type_acquisition_option_0
},
{
name: "extends",
type: "string"
}
},
{
name: "include",
type: "list",
element: {
},
{
name: "files",
type: "list",
element: {
name: "files",
type: "string"
}
},
{
name: "include",
type: "string"
}
},
{
name: "exclude",
type: "list",
element: {
type: "list",
element: {
name: "include",
type: "string"
}
},
{
name: "exclude",
type: "string"
}
},
compileOnSaveCommandLineOption
]);
type: "list",
element: {
name: "exclude",
type: "string"
}
},
compileOnSaveCommandLineOption
])
};
}
return _tsconfigRootOptions;
}
Expand Down Expand Up @@ -1061,31 +1071,38 @@ namespace ts {
* Convert the json syntax tree into the json value
*/
export function convertToObject(sourceFile: JsonSourceFile, errors: Push<Diagnostic>): any {
return convertToObjectWorker(sourceFile, errors, /*knownRootOptions*/ undefined, /*jsonConversionNotifier*/ undefined);
return convertToObjectWorker(sourceFile, errors, /*returnValue*/ true, /*knownRootOptions*/ undefined, /*jsonConversionNotifier*/ undefined);
}

/**
* Convert the json syntax tree into the json value
* Convert the json syntax tree into the json value and report errors
* This returns the json value (apart from checking errors) only if returnValue provided is true.
* Otherwise it just checks the errors and returns undefined
*/
function convertToObjectWorker(
/*@internal*/
export function convertToObjectWorker(
sourceFile: JsonSourceFile,
errors: Push<Diagnostic>,
knownRootOptions: Map<CommandLineOption> | undefined,
returnValue: boolean,
knownRootOptions: CommandLineOption | undefined,
jsonConversionNotifier: JsonConversionNotifier | undefined): any {
if (!sourceFile.jsonObject) {
return {};
if (!sourceFile.statements.length) {
return returnValue ? {} : undefined;
}

return convertObjectLiteralExpressionToJson(sourceFile.jsonObject, knownRootOptions,
/*extraKeyDiagnosticMessage*/ undefined, /*parentOption*/ undefined);
return convertPropertyValueToJson(sourceFile.statements[0].expression, knownRootOptions);

function isRootOptionMap(knownOptions: Map<CommandLineOption> | undefined) {
return knownRootOptions && (knownRootOptions as TsConfigOnlyOption).elementOptions === knownOptions;
}

function convertObjectLiteralExpressionToJson(
node: ObjectLiteralExpression,
knownOptions: Map<CommandLineOption> | undefined,
extraKeyDiagnosticMessage: DiagnosticMessage | undefined,
parentOption: string | undefined
): any {
const result: any = {};
const result: any = returnValue ? {} : undefined;
Copy link
Member

Choose a reason for hiding this comment

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

is this returnValue check just for efficiency, to avoid building up a value that's not needed.

Actually, why isn't it needed, when existing callers needed it?

Copy link
Member Author

Choose a reason for hiding this comment

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

We dont need value when we are importing modules: We just want the error reporting on valid json syntax. So creating (potentially large) json object isnt needed hence the flag. In case of config file, we do need json object as well as checks which can be done simultaneously.

Copy link

Choose a reason for hiding this comment

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

Could you add a comment on the function header?

for (const element of node.properties) {
if (element.kind !== SyntaxKind.PropertyAssignment) {
errors.push(createDiagnosticForNodeInSourceFile(sourceFile, element, Diagnostics.Property_assignment_expected));
Expand All @@ -1106,19 +1123,21 @@ namespace ts {
}
const value = convertPropertyValueToJson(element.initializer, option);
if (typeof keyText !== "undefined") {
result[keyText] = value;
if (returnValue) {
result[keyText] = value;
}
// Notify key value set, if user asked for it
if (jsonConversionNotifier &&
// Current callbacks are only on known parent option or if we are setting values in the root
(parentOption || knownOptions === knownRootOptions)) {
(parentOption || isRootOptionMap(knownOptions))) {
const isValidOptionValue = isCompilerOptionsValue(option, value);
if (parentOption) {
if (isValidOptionValue) {
// Notify option set in the parent if its a valid option value
jsonConversionNotifier.onSetValidOptionKeyValueInParent(parentOption, option, value);
}
}
else if (knownOptions === knownRootOptions) {
else if (isRootOptionMap(knownOptions)) {
if (isValidOptionValue) {
// Notify about the valid root key value being set
jsonConversionNotifier.onSetValidOptionKeyValueInRoot(keyText, element.name, value, element.initializer);
Expand All @@ -1137,8 +1156,8 @@ namespace ts {
function convertArrayLiteralExpressionToJson(
elements: NodeArray<Expression>,
elementOption: CommandLineOption | undefined
): any[] {
return elements.map(element => convertPropertyValueToJson(element, elementOption));
): any[] | void {
return (returnValue ? elements.map : elements.forEach).call(elements, (element: Expression) => convertPropertyValueToJson(element, elementOption));
}

function convertPropertyValueToJson(valueExpression: Expression, option: CommandLineOption): any {
Expand Down Expand Up @@ -1433,12 +1452,12 @@ namespace ts {
* @param basePath A root directory to resolve relative path entries in the config
* file to. e.g. outDir
*/
export function parseJsonSourceFileConfigFileContent(sourceFile: JsonSourceFile, host: ParseConfigHost, basePath: string, existingOptions?: CompilerOptions, configFileName?: string, resolutionStack?: Path[], extraFileExtensions?: ReadonlyArray<FileExtensionInfo>): ParsedCommandLine {
export function parseJsonSourceFileConfigFileContent(sourceFile: TsConfigSourceFile, host: ParseConfigHost, basePath: string, existingOptions?: CompilerOptions, configFileName?: string, resolutionStack?: Path[], extraFileExtensions?: ReadonlyArray<FileExtensionInfo>): ParsedCommandLine {
return parseJsonConfigFileContentWorker(/*json*/ undefined, sourceFile, host, basePath, existingOptions, configFileName, resolutionStack, extraFileExtensions);
}

/*@internal*/
export function setConfigFileInOptions(options: CompilerOptions, configFile: JsonSourceFile) {
export function setConfigFileInOptions(options: CompilerOptions, configFile: TsConfigSourceFile) {
if (configFile) {
Object.defineProperty(options, "configFile", { enumerable: false, writable: false, value: configFile });
}
Expand Down Expand Up @@ -1466,7 +1485,7 @@ namespace ts {
*/
function parseJsonConfigFileContentWorker(
json: any,
sourceFile: JsonSourceFile,
sourceFile: TsConfigSourceFile,
host: ParseConfigHost,
basePath: string,
existingOptions: CompilerOptions = {},
Expand Down Expand Up @@ -1587,7 +1606,7 @@ namespace ts {
*/
function parseConfig(
json: any,
sourceFile: JsonSourceFile,
sourceFile: TsConfigSourceFile,
host: ParseConfigHost,
basePath: string,
configFileName: string,
Expand Down Expand Up @@ -1664,7 +1683,7 @@ namespace ts {
}

function parseOwnConfigOfJsonSourceFile(
sourceFile: JsonSourceFile,
sourceFile: TsConfigSourceFile,
host: ParseConfigHost,
basePath: string,
configFileName: string | undefined,
Expand Down Expand Up @@ -1711,7 +1730,7 @@ namespace ts {
}
}
};
const json = convertToObjectWorker(sourceFile, errors, getTsconfigRootOptionsMap(), optionsIterator);
const json = convertToObjectWorker(sourceFile, errors, /*returnValue*/ true, getTsconfigRootOptionsMap(), optionsIterator);
if (!typeAcquisition) {
if (typingOptionstypeAcquisition) {
typeAcquisition = (typingOptionstypeAcquisition.enableAutoDiscovery !== undefined) ?
Expand Down Expand Up @@ -1754,7 +1773,7 @@ namespace ts {
}

function getExtendedConfig(
sourceFile: JsonSourceFile,
sourceFile: TsConfigSourceFile,
extendedConfigPath: string,
host: ParseConfigHost,
basePath: string,
Expand Down Expand Up @@ -2006,7 +2025,7 @@ namespace ts {
host: ParseConfigHost,
errors: Push<Diagnostic>,
extraFileExtensions: ReadonlyArray<FileExtensionInfo>,
jsonSourceFile: JsonSourceFile
jsonSourceFile: TsConfigSourceFile
): ExpandResult {
basePath = normalizePath(basePath);
let validatedIncludeSpecs: ReadonlyArray<string>, validatedExcludeSpecs: ReadonlyArray<string>;
Expand Down Expand Up @@ -2107,7 +2126,7 @@ namespace ts {
};
}

function validateSpecs(specs: ReadonlyArray<string>, errors: Push<Diagnostic>, allowTrailingRecursion: boolean, jsonSourceFile: JsonSourceFile, specKey: string): ReadonlyArray<string> {
function validateSpecs(specs: ReadonlyArray<string>, errors: Push<Diagnostic>, allowTrailingRecursion: boolean, jsonSourceFile: TsConfigSourceFile, specKey: string): ReadonlyArray<string> {
return specs.filter(spec => {
const diag = specToDiagnostic(spec, allowTrailingRecursion);
if (diag !== undefined) {
Expand All @@ -2117,18 +2136,10 @@ namespace ts {
});

function createDiagnostic(message: DiagnosticMessage, spec: string): Diagnostic {
if (jsonSourceFile && jsonSourceFile.jsonObject) {
for (const property of getPropertyAssignment(jsonSourceFile.jsonObject, specKey)) {
if (isArrayLiteralExpression(property.initializer)) {
for (const element of property.initializer.elements) {
if (isStringLiteral(element) && element.text === spec) {
return createDiagnosticForNodeInSourceFile(jsonSourceFile, element, message, spec);
}
}
}
}
}
return createCompilerDiagnostic(message, spec);
const element = getTsConfigPropArrayElementValue(jsonSourceFile, specKey, spec);
return element ?
createDiagnosticForNodeInSourceFile(jsonSourceFile, element, message, spec) :
createCompilerDiagnostic(message, spec);
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2966,7 +2966,7 @@ namespace ts {
}
}

const extensionsToRemove = [Extension.Dts, Extension.Ts, Extension.Js, Extension.Tsx, Extension.Jsx];
const extensionsToRemove = [Extension.Dts, Extension.Ts, Extension.Js, Extension.Tsx, Extension.Jsx, Extension.Json];
export function removeFileExtension(path: string): string {
for (const ext of extensionsToRemove) {
const extensionless = tryRemoveExtension(path, ext);
Expand Down Expand Up @@ -3307,6 +3307,10 @@ namespace ts {
return ext === Extension.Ts || ext === Extension.Tsx || ext === Extension.Dts;
}

export function resolutionExtensionIsTypeScriptOrJson(ext: Extension) {
return extensionIsTypeScript(ext) || ext === Extension.Json;
}

/**
* Gets the extension from a path.
* Path must have a valid extension.
Expand Down
Loading