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

feat: support subpath imports #13705

Merged
merged 5 commits into from
Jan 1, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

### Features

- `[jest-resolve]` Support subpath imports ([#13705](https://github.com/facebook/jest/pull/13705))
- `[jest-runtime]` Add `jest.isolateModulesAsync` for scoped module initialization of asynchronous functions ([#13680](https://github.com/facebook/jest/pull/13680))
- `[jest-test-result]` Added `skipped` and `focused` status to `FormattedTestResult` ([#13700](https://github.com/facebook/jest/pull/13700))

Expand Down
4 changes: 2 additions & 2 deletions e2e/__tests__/__snapshots__/moduleNameMapper.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ exports[`moduleNameMapper wrong array configuration 1`] = `
12 | module.exports = () => 'test';
13 |

at createNoMappedModuleFoundError (../../packages/jest-resolve/build/resolver.js:758:17)
at createNoMappedModuleFoundError (../../packages/jest-resolve/build/resolver.js:760:17)
at Object.require (index.js:10:1)
at Object.require (__tests__/index.js:10:20)"
`;
Expand Down Expand Up @@ -71,7 +71,7 @@ exports[`moduleNameMapper wrong configuration 1`] = `
12 | module.exports = () => 'test';
13 |

at createNoMappedModuleFoundError (../../packages/jest-resolve/build/resolver.js:758:17)
at createNoMappedModuleFoundError (../../packages/jest-resolve/build/resolver.js:760:17)
at Object.require (index.js:10:1)
at Object.require (__tests__/index.js:10:20)"
`;
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/requireMissingExt.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ exports[`shows a proper error from deep requires 1`] = `
12 | test('dummy', () => {
13 | expect(1).toBe(1);

at Resolver._throwModNotFoundError (../../packages/jest-resolve/build/resolver.js:425:11)
at Resolver._throwModNotFoundError (../../packages/jest-resolve/build/resolver.js:427:11)
at Object.<anonymous> (node_modules/discord.js/src/index.js:21:12)
at Object.require (__tests__/test.js:10:1)"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ exports[`show error message with matching files 1`] = `
| ^
9 |

at Resolver._throwModNotFoundError (../../packages/jest-resolve/build/resolver.js:425:11)
at Resolver._throwModNotFoundError (../../packages/jest-resolve/build/resolver.js:427:11)
at Object.require (index.js:8:18)
at Object.require (__tests__/test.js:8:11)"
`;
1 change: 1 addition & 0 deletions packages/jest-resolve/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"./package.json": "./package.json"
},
"dependencies": {
"@okikio/resolve.imports": "^1.0.0",
"chalk": "^4.0.0",
"graceful-fs": "^4.2.9",
"jest-haste-map": "workspace:^",
Expand Down
Empty file.

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "foo-import",
"imports": {
"#nested": {
"require": "./internal.cjs",
"default": "external-foo"
}
}
}
3 changes: 3 additions & 0 deletions packages/jest-resolve/src/__mocks__/imports/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "imports"
}
38 changes: 38 additions & 0 deletions packages/jest-resolve/src/__tests__/resolve.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,44 @@ describe('findNodeModule', () => {
expect(result).toBeNull();
});
});

describe('imports', () => {
const importsRoot = path.resolve(__dirname, '../__mocks__/imports');

test('supports internal reference', () => {
const result = Resolver.findNodeModule('#nested', {
basedir: path.resolve(importsRoot, './foo-import/index.cjs'),
conditions: ['require'],
});

expect(result).toEqual(
path.resolve(importsRoot, './foo-import/internal.cjs'),
);
});

test('supports external reference', () => {
const result = Resolver.findNodeModule('#nested', {
basedir: path.resolve(importsRoot, './foo-import/index.js'),
conditions: [],
});

expect(result).toEqual(
path.resolve(
importsRoot,
'./foo-import/node_modules/external-foo/main.js',
),
);
});

test('fails for non-existent mapping', () => {
expect(() => {
Resolver.findNodeModule('#something-else', {
basedir: path.resolve(importsRoot, './foo-import/index.js'),
conditions: [],
});
}).toThrow('Missing "#something-else" import in "foo-import" package');
});
});
});

describe('findNodeModuleAsync', () => {
Expand Down
37 changes: 34 additions & 3 deletions packages/jest-resolve/src/defaultResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import {dirname, isAbsolute, resolve as pathResolve} from 'path';
import {resolve as resolveImports} from '@okikio/resolve.imports';
import pnpResolver from 'jest-pnp-resolver';
import {SyncOpts as UpstreamResolveOptions, sync as resolveSync} from 'resolve';
import {
Expand Down Expand Up @@ -83,7 +84,7 @@ export type ResolverOptions = {
};

type UpstreamResolveOptionsWithConditions = UpstreamResolveOptions &
Pick<ResolverOptions, 'basedir' | 'conditions'>;
ResolverOptions;

export type SyncResolver = (path: string, options: ResolverOptions) => string;
export type AsyncResolver = (
Expand Down Expand Up @@ -136,12 +137,42 @@ function getPathInModule(
return path;
}

if (path.startsWith('#')) {
const closestPackageJson = findClosestPackageJson(options.basedir);

if (!closestPackageJson) {
throw new Error(
`Jest: unable to locate closest package.json from ${options.basedir} when resolving import "${path}"`,
);
}

const pkg = readPackageCached(closestPackageJson);

const resolved = resolveImports(
pkg,
path,
createResolveOptions(options.conditions),
);
Comment on lines +151 to +155
Copy link
Member Author

@SimenB SimenB Jan 1, 2023

Choose a reason for hiding this comment

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

this throws on missing imports etc, so we don't need to check if it's present in pkg


if (!resolved) {
throw new Error(
'`imports` exists, but no results - this is a bug in Jest. Please report an issue',
);
}

if (resolved.startsWith('.')) {
return pathResolve(dirname(closestPackageJson), resolved);
}

// this is an external module, re-resolve it
return defaultResolver(resolved, options);
}

const segments = path.split('/');

let moduleName = segments.shift();

if (moduleName) {
// TODO: handle `#` here: https://github.com/facebook/jest/issues/12270
if (moduleName.startsWith('@')) {
moduleName = `${moduleName}/${segments.shift()}`;
}
Expand Down Expand Up @@ -213,6 +244,6 @@ function createResolveOptions(
{browser: false, require: true};
}

// if it's a relative import or an absolute path, exports are ignored
// if it's a relative import or an absolute path, imports/exports are ignored
const shouldIgnoreRequestForExports = (path: string) =>
path.startsWith('.') || isAbsolute(path);
6 changes: 4 additions & 2 deletions packages/jest-resolve/src/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ export default class Resolver {
rootDir: options.rootDir,
});
} catch (e) {
if (options.throwIfNotFound) {
// we always wanna throw if it's an internal import
if (options.throwIfNotFound || path.startsWith('#')) {
Comment on lines +134 to +135
Copy link
Member Author

Choose a reason for hiding this comment

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

no reason to ever not throw this directly

throw e;
}
}
Expand Down Expand Up @@ -174,7 +175,8 @@ export default class Resolver {
});
return result;
} catch (e: unknown) {
if (options.throwIfNotFound) {
// we always wanna throw if it's an internal import
if (options.throwIfNotFound || path.startsWith('#')) {
throw e;
}
}
Expand Down
8 changes: 8 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3757,6 +3757,13 @@ __metadata:
languageName: node
linkType: hard

"@okikio/resolve.imports@npm:^1.0.0":
version: 1.0.0
resolution: "@okikio/resolve.imports@npm:1.0.0"
checksum: a06d347731b3c47e79125d346dd0172fda3cf20d138249f4ed23c5cc60b32f668f9bbab49698fc061ef5782d236622ff4cd0ce2b2520550273e863f55b687350
languageName: node
linkType: hard

"@pkgr/utils@npm:^2.3.1":
version: 2.3.1
resolution: "@pkgr/utils@npm:2.3.1"
Expand Down Expand Up @@ -12918,6 +12925,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "jest-resolve@workspace:packages/jest-resolve"
dependencies:
"@okikio/resolve.imports": ^1.0.0
"@tsd/typescript": ^4.9.0
"@types/graceful-fs": ^4.1.3
"@types/pnpapi": ^0.0.2
Expand Down