Skip to content

Commit

Permalink
Fix #1778: typechecker resolver should take importer's module type --…
Browse files Browse the repository at this point in the history
… cjs or esm -- into account when resolving package.json "exports" (#1782)

* Add failing test

* fix typo in failing test

* fix bug

* fix test runIf filter

* fix

* fix

* lint
  • Loading branch information
cspotcode committed Jun 1, 2022
1 parent b3dd3f2 commit 04f4c28
Show file tree
Hide file tree
Showing 13 changed files with 111 additions and 30 deletions.
20 changes: 15 additions & 5 deletions src/resolver-functions.ts
Expand Up @@ -95,16 +95,24 @@ export function createResolverFunctions(kwargs: {
containingFile: string,
reusedNames: string[] | undefined,
redirectedReference: TSCommon.ResolvedProjectReference | undefined,
optionsOnlyWithNewerTsVersions: TSCommon.CompilerOptions
optionsOnlyWithNewerTsVersions: TSCommon.CompilerOptions,
containingSourceFile?: TSCommon.SourceFile
): (TSCommon.ResolvedModule | undefined)[] => {
return moduleNames.map((moduleName) => {
return moduleNames.map((moduleName, i) => {
const mode = containingSourceFile
? (ts as any as TSInternal).getModeForResolutionAtIndex?.(
containingSourceFile,
i
)
: undefined;
const { resolvedModule } = ts.resolveModuleName(
moduleName,
containingFile,
config.options,
host,
moduleResolutionCache,
redirectedReference
redirectedReference,
mode
);
if (resolvedModule) {
fixupResolvedModule(resolvedModule);
Expand All @@ -117,12 +125,14 @@ export function createResolverFunctions(kwargs: {
const getResolvedModuleWithFailedLookupLocationsFromCache: TSCommon.LanguageServiceHost['getResolvedModuleWithFailedLookupLocationsFromCache'] =
(
moduleName,
containingFile
containingFile,
resolutionMode?: TSCommon.ModuleKind.CommonJS | TSCommon.ModuleKind.ESNext
): TSCommon.ResolvedModuleWithFailedLookupLocations | undefined => {
const ret = ts.resolveModuleNameFromCache(
moduleName,
containingFile,
moduleResolutionCache
moduleResolutionCache,
resolutionMode
);
if (ret && ret.resolvedModule) {
fixupResolvedModule(ret.resolvedModule);
Expand Down
38 changes: 38 additions & 0 deletions src/test/module-node/1778.spec.ts
@@ -0,0 +1,38 @@
import { createExec } from '../exec-helpers';
import {
ctxTsNode,
nodeSupportsEsmHooks,
TEST_DIR,
tsSupportsStableNodeNextNode16,
CMD_TS_NODE_WITHOUT_PROJECT_FLAG,
nodeSupportsSpawningChildProcess,
} from '../helpers';
import { context, expect } from '../testlib';
import { join } from 'path';

const exec = createExec({
cwd: TEST_DIR,
});

const test = context(ctxTsNode);

test.suite(
'Issue #1778: typechecker resolver should take importer\'s module type -- cjs or esm -- into account when resolving package.json "exports"',
(test) => {
test.runIf(
nodeSupportsEsmHooks &&
nodeSupportsSpawningChildProcess &&
tsSupportsStableNodeNextNode16
);
test('test', async () => {
const { err, stdout } = await exec(
`${CMD_TS_NODE_WITHOUT_PROJECT_FLAG} ./index.ts`,
{
cwd: join(TEST_DIR, '1778'),
}
);
expect(err).toBe(null);
expect(stdout).toBe('{ esm: true }\n');
});
}
);
@@ -1,15 +1,15 @@
import { expect, context } from './testlib';
import { expect, context } from '../testlib';
import {
CMD_TS_NODE_WITHOUT_PROJECT_FLAG,
isOneOf,
nodeSupportsImportingTransformedCjsFromEsm,
resetNodeEnvironment,
tsSupportsStableNodeNextNode16,
} from './helpers';
} from '../helpers';
import * as Path from 'path';
import { ctxTsNode } from './helpers';
import { exec } from './exec-helpers';
import { file, project, ProjectAPI as ProjectAPI } from './fs-helpers';
import { ctxTsNode } from '../helpers';
import { exec } from '../exec-helpers';
import { file, project, ProjectAPI as ProjectAPI } from '../fs-helpers';

const test = context(ctxTsNode);
test.beforeEach(async () => {
Expand Down
37 changes: 17 additions & 20 deletions src/ts-compiler-types.ts
Expand Up @@ -32,16 +32,7 @@ export interface TSCommon {
createModuleResolutionCache: typeof _ts.createModuleResolutionCache;
resolveModuleName: typeof _ts.resolveModuleName;
resolveModuleNameFromCache: typeof _ts.resolveModuleNameFromCache;
// Changed in TS 4.7
resolveTypeReferenceDirective(
typeReferenceDirectiveName: string,
containingFile: string | undefined,
options: _ts.CompilerOptions,
host: _ts.ModuleResolutionHost,
redirectedReference?: _ts.ResolvedProjectReference,
cache?: _ts.TypeReferenceDirectiveResolutionCache,
resolutionMode?: _ts.SourceFile['impliedNodeFormat']
): _ts.ResolvedTypeReferenceDirectiveWithFailedLookupLocations;
resolveTypeReferenceDirective: typeof _ts.resolveTypeReferenceDirective;
createIncrementalCompilerHost: typeof _ts.createIncrementalCompilerHost;
createSourceFile: typeof _ts.createSourceFile;
getDefaultLibFileName: typeof _ts.getDefaultLibFileName;
Expand All @@ -52,16 +43,7 @@ export interface TSCommon {
ModuleResolutionKind: typeof _ts.ModuleResolutionKind;
}
export namespace TSCommon {
export interface LanguageServiceHost extends _ts.LanguageServiceHost {
// Modified in 4.7
resolveTypeReferenceDirectives?(
typeDirectiveNames: string[] | _ts.FileReference[],
containingFile: string,
redirectedReference: _ts.ResolvedProjectReference | undefined,
options: _ts.CompilerOptions,
containingFileMode?: _ts.SourceFile['impliedNodeFormat'] | undefined
): (_ts.ResolvedTypeReferenceDirective | undefined)[];
}
export interface LanguageServiceHost extends _ts.LanguageServiceHost {}
export type ModuleResolutionHost = _ts.ModuleResolutionHost;
export type ParsedCommandLine = _ts.ParsedCommandLine;
export type ResolvedModule = _ts.ResolvedModule;
Expand All @@ -79,6 +61,12 @@ export namespace TSCommon {
? typeof _ts.ModuleKind['Node16']
: 100;
};
// Can't figure out how to re-export an enum
// `export import ... =` complains that _ts is type-only import
export namespace ModuleKind {
export type CommonJS = _ts.ModuleKind.CommonJS;
export type ESNext = _ts.ModuleKind.ESNext;
}
}

/**
Expand Down Expand Up @@ -129,6 +117,11 @@ export interface TSInternal {
basePath: string,
usage: 'files' | 'directories' | 'exclude'
): string | undefined;
// Added in TS 4.7
getModeForResolutionAtIndex?(
file: TSInternal.SourceFileImportsList,
index: number
): _ts.SourceFile['impliedNodeFormat'];
}
/** @internal */
export namespace TSInternal {
Expand All @@ -139,4 +132,8 @@ export namespace TSInternal {
getCurrentDirectory(): string;
useCaseSensitiveFileNames: boolean;
}
// Note: is only a partial declaration, TS sources declare other fields
export interface SourceFileImportsList {
impliedNodeFormat?: TSCommon.SourceFile['impliedNodeFormat'];
}
}
6 changes: 6 additions & 0 deletions tests/1778/index.ts
@@ -0,0 +1,6 @@
import foo from 'foo';

// This file is ESM, so if typechecker's resolver is working correctly, will
// resolve to the foo's package.json "exports" mapping for "default", not "require"
const bar: { esm: true } = foo;
console.log(bar);
2 changes: 2 additions & 0 deletions tests/1778/node_modules/foo/cjs/index.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/1778/node_modules/foo/cjs/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions tests/1778/node_modules/foo/cjs/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions tests/1778/node_modules/foo/esm/index.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions tests/1778/node_modules/foo/esm/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions tests/1778/node_modules/foo/package.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions tests/1778/package.json
@@ -0,0 +1,3 @@
{
"type": "module"
}
9 changes: 9 additions & 0 deletions tests/1778/tsconfig.json
@@ -0,0 +1,9 @@
{
"ts-node": {
"esm": true
},
"compilerOptions": {
"module": "NodeNext",
"noEmit": true
}
}

0 comments on commit 04f4c28

Please sign in to comment.