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.

Sign up for GitHub

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

chore: migrate jest-resolve to TypeScript #7871

Merged
merged 1 commit into from
Feb 13, 2019
Merged

chore: migrate jest-resolve to TypeScript #7871

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
- `[jest-haste-map]`: Migrate to TypeScript ([#7854](https://github.com/facebook/jest/pull/7854))
- `[docs]`: Fix image paths in SnapshotTesting.md for current and version 24 ([#7872](https://github.com/facebook/jest/pull/7872))
- `[babel-jest]`: Migrate to TypeScript ([#7862](https://github.com/facebook/jest/pull/7862))
- `[jest-resolve]`: Migrate to TypeScript ([#7871](https://github.com/facebook/jest/pull/7871))

### Performance

Expand Down
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/moduleNameMapper.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ FAIL __tests__/index.js
12 | module.exports = () => 'test';
13 |

at createNoMappedModuleFoundError (../../packages/jest-resolve/build/index.js:436:17)
at createNoMappedModuleFoundError (../../packages/jest-resolve/build/index.js:434:17)
at Object.require (index.js:10:1)
`;
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/resolveNoFileExtensions.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ FAIL __tests__/test.js
| ^
4 |

at Resolver.resolveModule (../../packages/jest-resolve/build/index.js:203:17)
at Resolver.resolveModule (../../packages/jest-resolve/build/index.js:201:17)
at Object.require (index.js:3:18)
`;
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
"progress": "^2.0.0",
"promise": "^8.0.2",
"readable-stream": "^3.0.3",
"realpath-native": "^1.0.0",
"realpath-native": "^1.1.0",
thymikee marked this conversation as resolved.
Show resolved Hide resolved
"resolve": "^1.4.0",
"rimraf": "^2.6.2",
"slash": "^2.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"p-each-series": "^1.0.0",
"pirates": "^4.0.0",
"prompts": "^2.0.1",
"realpath-native": "^1.0.0",
"realpath-native": "^1.1.0",
"rimraf": "^2.5.4",
"slash": "^2.0.0",
"string-length": "^2.0.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"jest-validate": "^24.0.0",
"micromatch": "^3.1.10",
"pretty-format": "^24.0.0",
"realpath-native": "^1.0.2"
"realpath-native": "^1.1.0"
},
"devDependencies": {
"@types/babel__core": "^7.0.4",
Expand Down
10 changes: 5 additions & 5 deletions packages/jest-haste-map/src/ModuleMap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ export default class ModuleMap {

getModule(
name: string,
platform: string | null,
supportsNativePlatform: boolean | null,
type: HTypeValue | null,
platform?: string | null,
SimenB marked this conversation as resolved.
Show resolved Hide resolved
supportsNativePlatform?: boolean | null,
type?: HTypeValue | null,
): Config.Path | null {
if (type == null) {
type = H.MODULE;
Expand All @@ -62,7 +62,7 @@ export default class ModuleMap {

getPackage(
name: string,
platform: string | null,
platform: string | null | undefined,
_supportsNativePlatform: boolean | null,
): Config.Path | null {
return this.getModule(name, platform, null, H.PACKAGE);
Expand Down Expand Up @@ -111,7 +111,7 @@ export default class ModuleMap {
*/
private _getModuleMetadata(
name: string,
platform: string | null,
platform: string | null | undefined,
supportsNativePlatform: boolean,
): ModuleMetaData | null {
const map = this._raw.map.get(name) || EMPTY_OBJ;
Expand Down
7 changes: 6 additions & 1 deletion packages/jest-resolve/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,20 @@
},
"license": "MIT",
"main": "build/index.js",
"types": "build/index.d.ts",
"dependencies": {
"@jest/types": "^24.1.0",
"browser-resolve": "^1.11.3",
"chalk": "^2.0.1",
"realpath-native": "^1.0.0"
"realpath-native": "^1.1.0"
},
"devDependencies": {
"@types/browser-resolve": "^0.0.5",
"jest-haste-map": "^24.0.0"
},
"peerDependencies": {
"jest-haste-map": "^24.0.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this makes the most sense - the constructor of jest-resolve takes ModuleMap from jest-haste-map. So both for types but also for usage, we need to ensure that the API is what we expect it to be

},
"engines": {
"node": ">= 6"
},
Expand Down
1 change: 0 additions & 1 deletion ...lve/src/__tests__/isBuiltinModule.test.js → ...lve/src/__tests__/isBuiltinModule.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
// Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
// @flow

import isBuiltinModule from '../isBuiltinModule';

Expand Down
34 changes: 18 additions & 16 deletions ...est-resolve/src/__tests__/resolve.test.js → ...est-resolve/src/__tests__/resolve.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@
*
*/

'use strict';

import fs from 'fs';
import path from 'path';
import {ModuleMap} from 'jest-haste-map';
// eslint-disable-next-line import/default
import HasteMap from 'jest-haste-map';
import Resolver from '../';
// @ts-ignore: js file
import userResolver from '../__mocks__/userResolver';
import nodeModulesPaths from '../nodeModulesPaths';
import defaultResolver from '../defaultResolver';
import {ResolverConfig} from '../types';

// @ts-ignore: types are wrong. not sure how...
Copy link
Contributor

Choose a reason for hiding this comment

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

built haste-map index.d.ts:

import HasteFS from './HasteFS';
import HasteModuleMap, { SerializableModuleMap as HasteSerializableModuleMap } from './ModuleMap';
export declare type ModuleMap = HasteModuleMap;
export declare type SerializableModuleMap = HasteSerializableModuleMap;
export declare type FS = HasteFS;

totally missing the main export.
Working on how to fix this atm

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we can fix this without dropping the export types at the top of haste map. Basically, we want both export = HasteMap (because making it the default export like it should be would be breaking) and export type .... It would be fine for a normal property (can just put it on the class), but I don't know how we could possibly put types onto HasteMap

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for digging! I say we keep it as is then, and try to go for a real ESM run at some point in the future.

It's easier with modules like haste map anyways since it's mostly internal and not pluggable like other parts of Jest are

Copy link
Contributor

Choose a reason for hiding this comment

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

Guess the ModuleMap and HasteFS types are more important anyway, they're the ones used in Context etc

const {ModuleMap} = HasteMap;

jest.mock('../__mocks__/userResolver');

Expand All @@ -28,21 +30,21 @@ describe('isCoreModule', () => {
const moduleMap = ModuleMap.create('/');
const resolver = new Resolver(moduleMap, {
hasCoreModules: false,
});
} as ResolverConfig);
const isCore = resolver.isCoreModule('assert');
expect(isCore).toEqual(false);
});

it('returns true if `hasCoreModules` is true and `moduleName` is a core module.', () => {
const moduleMap = ModuleMap.create('/');
const resolver = new Resolver(moduleMap, {});
const resolver = new Resolver(moduleMap, {} as ResolverConfig);
const isCore = resolver.isCoreModule('assert');
expect(isCore).toEqual(true);
});

it('returns false if `hasCoreModules` is true and `moduleName` is not a core module.', () => {
const moduleMap = ModuleMap.create('/');
const resolver = new Resolver(moduleMap, {});
const resolver = new Resolver(moduleMap, {} as ResolverConfig);
const isCore = resolver.isCoreModule('not-a-core-module');
expect(isCore).toEqual(false);
});
Expand Down Expand Up @@ -84,15 +86,15 @@ describe('findNodeModule', () => {
});

describe('resolveModule', () => {
let moduleMap;
let moduleMap: typeof ModuleMap;
beforeEach(() => {
moduleMap = ModuleMap.create('/');
});

it('is possible to resolve node modules', () => {
const resolver = new Resolver(moduleMap, {
extensions: ['.js'],
});
} as ResolverConfig);
const src = require.resolve('../');
const resolved = resolver.resolveModule(
src,
Expand All @@ -104,7 +106,7 @@ describe('resolveModule', () => {
it('is possible to resolve node modules with custom extensions', () => {
const resolver = new Resolver(moduleMap, {
extensions: ['.js', '.jsx'],
});
} as ResolverConfig);
const src = require.resolve('../');
const resolvedJsx = resolver.resolveModule(
src,
Expand All @@ -119,7 +121,7 @@ describe('resolveModule', () => {
const resolver = new Resolver(moduleMap, {
extensions: ['.js', '.jsx'],
platforms: ['native'],
});
} as ResolverConfig);
const src = require.resolve('../');
const resolvedJsx = resolver.resolveModule(
src,
Expand All @@ -133,7 +135,7 @@ describe('resolveModule', () => {
it('is possible to resolve node modules by resolving their realpath', () => {
const resolver = new Resolver(moduleMap, {
extensions: ['.js'],
});
} as ResolverConfig);
const src = path.join(
path.resolve(__dirname, '../../src/__mocks__/bar/node_modules/'),
'foo/index.js',
Expand All @@ -147,7 +149,7 @@ describe('resolveModule', () => {
it('is possible to specify custom resolve paths', () => {
const resolver = new Resolver(moduleMap, {
extensions: ['.js'],
});
} as ResolverConfig);
const src = require.resolve('../');
const resolved = resolver.resolveModule(src, 'mockJsDependency', {
paths: [
Expand All @@ -173,7 +175,7 @@ describe('getMockModule', () => {
},
],
resolver: require.resolve('../__mocks__/userResolver'),
});
} as ResolverConfig);
const src = require.resolve('../');
resolver.getMockModule(src, 'dependentModule');

Expand All @@ -196,7 +198,7 @@ describe('nodeModulesPaths', () => {

describe('Resolver.getModulePaths() -> nodeModulesPaths()', () => {
const _path = path;
let moduleMap;
let moduleMap: typeof ModuleMap;

beforeEach(() => {
jest.resetModules();
Expand All @@ -208,7 +210,7 @@ describe('Resolver.getModulePaths() -> nodeModulesPaths()', () => {
// This test suite won't work otherwise, since we cannot make assumptions
// about the test environment when it comes to absolute paths.
jest.doMock('realpath-native', () => ({
sync: dirInput => dirInput,
sync: (dirInput: string) => dirInput,
}));
});

Expand Down
49 changes: 24 additions & 25 deletions packages/jest-resolve/src/defaultResolver.js → packages/jest-resolve/src/defaultResolver.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,29 @@
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

import type {Path} from 'types/Config';
import type {ErrorWithCode} from 'types/Errors';

import browserResolve from 'browser-resolve';
import fs from 'fs';
import path from 'path';
import browserResolve from 'browser-resolve';
import {Config} from '@jest/types';
import isBuiltinModule from './isBuiltinModule';
import nodeModulesPaths from './nodeModulesPaths';

type ResolverOptions = {|
basedir: Path,
browser?: boolean,
defaultResolver: typeof defaultResolver,
extensions?: Array<string>,
moduleDirectory?: Array<string>,
paths?: ?Array<Path>,
rootDir: ?Path,
|};
type ResolverOptions = {
basedir: Config.Path;
browser?: boolean;
defaultResolver: typeof defaultResolver;
extensions?: Array<string>;
moduleDirectory?: Array<string>;
paths?: Array<Config.Path>;
rootDir?: Config.Path;
};

export default function defaultResolver(
path: Path,
path: Config.Path,
options: ResolverOptions,
): Path {
): Config.Path {
const resolve = options.browser ? browserResolve.sync : resolveSync;

return resolve(path, {
Expand All @@ -44,7 +40,10 @@ export default function defaultResolver(

const REGEX_RELATIVE_IMPORT = /^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[\\\/])/;

function resolveSync(target: Path, options: ResolverOptions): Path {
function resolveSync(
target: Config.Path,
options: ResolverOptions,
): Config.Path {
const basedir = options.basedir;
const extensions = options.extensions || ['.js'];
const paths = options.paths || [];
Expand Down Expand Up @@ -75,7 +74,7 @@ function resolveSync(target: Path, options: ResolverOptions): Path {
return target;
}

const err: ErrorWithCode = new Error(
const err: Error & {code?: string} = new Error(
"Cannot find module '" + target + "' from '" + basedir + "'",
);
err.code = 'MODULE_NOT_FOUND';
Expand All @@ -84,7 +83,7 @@ function resolveSync(target: Path, options: ResolverOptions): Path {
/*
* contextual helper functions
*/
function tryResolve(name: Path): ?Path {
function tryResolve(name: Config.Path): Config.Path | undefined {
const dir = path.dirname(name);
let result;
if (isDirectory(dir)) {
Expand All @@ -98,7 +97,7 @@ function resolveSync(target: Path, options: ResolverOptions): Path {
return result;
}

function resolveAsFile(name: Path): ?Path {
function resolveAsFile(name: Config.Path): Config.Path | undefined {
if (isFile(name)) {
return name;
}
Expand All @@ -113,7 +112,7 @@ function resolveSync(target: Path, options: ResolverOptions): Path {
return undefined;
}

function resolveAsDirectory(name: Path): ?Path {
function resolveAsDirectory(name: Config.Path): Config.Path | undefined {
if (!isDirectory(name)) {
return undefined;
}
Expand All @@ -140,7 +139,7 @@ function resolveSync(target: Path, options: ResolverOptions): Path {
/*
* helper functions
*/
function isFile(file: Path): boolean {
function isFile(file: Config.Path): boolean {
let result;

try {
Expand All @@ -156,7 +155,7 @@ function isFile(file: Path): boolean {
return result;
}

function isDirectory(dir: Path): boolean {
function isDirectory(dir: Config.Path): boolean {
let result;

try {
Expand All @@ -172,6 +171,6 @@ function isDirectory(dir: Path): boolean {
return result;
}

function isCurrentDirectory(testPath: Path): boolean {
function isCurrentDirectory(testPath: Config.Path): boolean {
return path.resolve('.') === path.resolve(testPath);
}
Loading