Skip to content

Dont emit module resolution errors when looking up specifiers for container symbols #28558

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

Merged
merged 1 commit into from
Nov 17, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2238,8 +2238,8 @@ namespace ts {
return initializer || decl;
}

function resolveExternalModuleName(location: Node, moduleReferenceExpression: Expression): Symbol | undefined {
return resolveExternalModuleNameWorker(location, moduleReferenceExpression, Diagnostics.Cannot_find_module_0);
function resolveExternalModuleName(location: Node, moduleReferenceExpression: Expression, ignoreErrors?: boolean): Symbol | undefined {
return resolveExternalModuleNameWorker(location, moduleReferenceExpression, ignoreErrors ? undefined : Diagnostics.Cannot_find_module_0);
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that supposed to be a link to the diff this comment is on? Because it seems to be.

Copy link
Member

Choose a reason for hiding this comment

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

Its line 2273 in the diff

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, naw that position always passes /*isError*/ false, so only manufactures suggestions, anyway. Given that it doesn't check the moduleNotFoundError flag, I assume the intent is to always generate the suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

So the suggestion is raised only when the container name? Seems flaky then.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it probably is. I assume it was written with the estimation that the module name is resolved for all imports, rather than only-imports-with-used-exports. I didn't add the suggestion, so I'm not sure myself, though.

}

function resolveExternalModuleNameWorker(location: Node, moduleReferenceExpression: Expression, moduleNotFoundError: DiagnosticMessage | undefined, isForAugmentation = false): Symbol | undefined {
Expand Down Expand Up @@ -2601,7 +2601,7 @@ namespace ts {
// Try to make an import using an import already in the enclosing file, if possible
for (const importRef of containingFile.imports) {
if (nodeIsSynthesized(importRef)) continue; // Synthetic names can't be resolved by `resolveExternalModuleName` - they'll cause a debug assert if they error
const resolvedModule = resolveExternalModuleName(enclosingDeclaration, importRef);
const resolvedModule = resolveExternalModuleName(enclosingDeclaration, importRef, /*ignoreErrors*/ true);
if (!resolvedModule) continue;
const ref = getAliasForSymbolInContainer(resolvedModule, symbol);
if (!ref) continue;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//// [jsxImportForSideEffectsNonExtantNoError.tsx]
/// <reference path="/.lib/react16.d.ts" />
import * as React from "react";

import "./App.css"; // doesn't actually exist

const tag = <div></div>;


//// [jsxImportForSideEffectsNonExtantNoError.js]
"use strict";
exports.__esModule = true;
/// <reference path="react16.d.ts" />
var React = require("react");
require("./App.css"); // doesn't actually exist
var tag = React.createElement("div", null);
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
=== tests/cases/compiler/jsxImportForSideEffectsNonExtantNoError.tsx ===
/// <reference path="react16.d.ts" />
import * as React from "react";
>React : Symbol(React, Decl(jsxImportForSideEffectsNonExtantNoError.tsx, 1, 6))

import "./App.css"; // doesn't actually exist

const tag = <div></div>;
>tag : Symbol(tag, Decl(jsxImportForSideEffectsNonExtantNoError.tsx, 5, 5))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))
>div : Symbol(JSX.IntrinsicElements.div, Decl(react16.d.ts, 2420, 114))

Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
=== tests/cases/compiler/jsxImportForSideEffectsNonExtantNoError.tsx ===
/// <reference path="react16.d.ts" />
import * as React from "react";
>React : typeof React

import "./App.css"; // doesn't actually exist

const tag = <div></div>;
>tag : JSX.Element
><div></div> : JSX.Element
>div : any
>div : any

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// @jsx: react
/// <reference path="/.lib/react16.d.ts" />
import * as React from "react";

import "./App.css"; // doesn't actually exist

const tag = <div></div>;