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

fix: mis-detecting imports on JavaScript when there is no checkJs #4040

Merged
merged 3 commits into from
Feb 20, 2020
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
16 changes: 9 additions & 7 deletions cli/js/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,6 @@ async function tsCompilerOnMessage({
type: CompilerRequestType[request.type]
});

// This will recursively analyse all the code for other imports,
// requesting those from the privileged side, populating the in memory
// cache which will be used by the host, before resolving.
const resolvedRootModules = await processImports(
rootNames.map(rootName => [rootName, rootName])
);

// When a programme is emitted, TypeScript will call `writeFile` with
// each file that needs to be emitted. The Deno compiler host delegates
// this, to make it easier to perform the right actions, which vary
Expand Down Expand Up @@ -141,6 +134,15 @@ async function tsCompilerOnMessage({
diagnostics = processConfigureResponse(configResult, configPath);
}

// This will recursively analyse all the code for other imports,
// requesting those from the privileged side, populating the in memory
// cache which will be used by the host, before resolving.
const resolvedRootModules = await processImports(
rootNames.map(rootName => [rootName, rootName]),
undefined,
host.getCompilationSettings().checkJs
);

let emitSkipped = true;
// if there was a configuration and no diagnostics with it, we will continue
// to generate the program and possibly emit it.
Expand Down
19 changes: 15 additions & 4 deletions cli/js/compiler_imports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ function getMediaType(filename: string): MediaType {
export function processLocalImports(
sources: Record<string, string>,
specifiers: Array<[string, string]>,
referrer?: string
referrer?: string,
checkJs = false
): string[] {
if (!specifiers.length) {
return [];
Expand All @@ -143,7 +144,12 @@ export function processLocalImports(
});
sourceFile.cache(specifiers[i][0], referrer);
if (!sourceFile.processed) {
processLocalImports(sources, sourceFile.imports(), sourceFile.url);
processLocalImports(
sources,
sourceFile.imports(checkJs),
sourceFile.url,
checkJs
);
}
}
return moduleNames;
Expand All @@ -157,7 +163,8 @@ export function processLocalImports(
* that should be actually resolved. */
export async function processImports(
specifiers: Array<[string, string]>,
referrer?: string
referrer?: string,
checkJs = false
): Promise<string[]> {
if (!specifiers.length) {
return [];
Expand All @@ -172,7 +179,11 @@ export async function processImports(
SourceFile.get(sourceFileJson.url) || new SourceFile(sourceFileJson);
sourceFile.cache(specifiers[i][0], referrer);
if (!sourceFile.processed) {
await processImports(sourceFile.imports(), sourceFile.url);
await processImports(
sourceFile.imports(checkJs),
sourceFile.url,
checkJs
);
}
}
return resolvedSources;
Expand Down
11 changes: 9 additions & 2 deletions cli/js/compiler_sourcefile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class SourceFile {
}

/** Process the imports for the file and return them. */
imports(): Array<[string, string]> {
imports(checkJs: boolean): Array<[string, string]> {
if (this.processed) {
throw new Error("SourceFile has already been processed.");
}
Expand All @@ -100,6 +100,7 @@ export class SourceFile {
log(`Skipping imports for "${this.filename}"`);
return [];
}

const preProcessedFileInfo = ts.preProcessFile(
this.sourceCode,
true,
Expand Down Expand Up @@ -129,7 +130,13 @@ export class SourceFile {
getMappedModuleName(importedFile, typeDirectives)
]);
}
} else {
} else if (
!(
!checkJs &&
(this.mediaType === MediaType.JavaScript ||
this.mediaType === MediaType.JSX)
)
) {
process(importedFiles);
}
process(referencedFiles);
Expand Down
3 changes: 3 additions & 0 deletions cli/tests/fix_js_imports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as amdLike from "./subdir/amd_like.js";

console.log(amdLike);
1 change: 1 addition & 0 deletions cli/tests/fix_js_imports.ts.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
5 changes: 5 additions & 0 deletions cli/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -954,6 +954,11 @@ itest!(cafile_info {
http_server: true,
});

itest!(fix_js_imports {
args: "run --reload fix_js_imports.ts",
output: "fix_js_imports.ts.out",
});

#[test]
fn cafile_fetch() {
pub use deno::test_util::*;
Expand Down
4 changes: 4 additions & 0 deletions cli/tests/subdir/amd_like.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// looks like an AMD module, but isn't
const define = () => {};
define(["fake_module"], () => {});
export {};