Skip to content

Commit

Permalink
Adjusting reverse-exports API
Browse files Browse the repository at this point in the history 
I want to use `@embroider/reverse-exports` for a new case where it's not an error to discover that a file is internal. So I would rather that it didn't throw in this case.
  • Loading branch information
@ef4
ef4 committed Aug 9, 2024
1 parent e9544fc commit d445aff
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 46 deletions.
22 changes: 17 additions & 5 deletions packages/core/src/module-resolver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type { Package, V2Package } from '@embroider/shared-internals';
import { explicitRelative, RewrittenPackageCache } from '@embroider/shared-internals';
import makeDebug from 'debug';
import assertNever from 'assert-never';
import reversePackageExports from '@embroider/reverse-exports';
import { externalName } from '@embroider/reverse-exports';
import { exports as resolveExports } from 'resolve.exports';

import {
Expand Down Expand Up @@ -810,13 +810,19 @@ export class Resolver {
`addon ${addon.name} declares app-js in its package.json with the illegal name "${inAddonName}". It must start with "./" to make it clear that it's relative to the addon`
);
}
let specifier = externalName(addon.packageJSON, inAddonName);
if (!specifier) {
throw new Error(
`${addon.name}'s package.json app-js refers to ${inAddonName}, but that module is not accessible from outside the package`
);
}
let prevEntry = engineModules.get(inEngineName);
switch (prevEntry?.type) {
case undefined:
engineModules.set(inEngineName, {
type: 'app-only',
'app-js': {
specifier: reversePackageExports(addon.packageJSON, inAddonName),
specifier,
fromFile: addonConfig.canResolveFromFile,
fromPackageName: addon.name,
},
Expand All @@ -830,7 +836,7 @@ export class Resolver {
engineModules.set(inEngineName, {
type: 'both',
'app-js': {
specifier: reversePackageExports(addon.packageJSON, inAddonName),
specifier,
fromFile: addonConfig.canResolveFromFile,
fromPackageName: addon.name,
},
Expand All @@ -854,13 +860,19 @@ export class Resolver {
`addon ${addon.name} declares fastboot-js in its package.json with the illegal name "${inAddonName}". It must start with "./" to make it clear that it's relative to the addon`
);
}
let specifier = externalName(addon.packageJSON, inAddonName);
if (!specifier) {
throw new Error(
`${addon.name}'s package.json fastboot-js refers to ${inAddonName}, but that module is not accessible from outside the package`
);
}
let prevEntry = engineModules.get(inEngineName);
switch (prevEntry?.type) {
case undefined:
engineModules.set(inEngineName, {
type: 'fastboot-only',
'fastboot-js': {
specifier: reversePackageExports(addon.packageJSON, inAddonName),
specifier,
fromFile: addonConfig.canResolveFromFile,
fromPackageName: addon.name,
},
Expand All @@ -874,7 +886,7 @@ export class Resolver {
engineModules.set(inEngineName, {
type: 'both',
'fastboot-js': {
specifier: reversePackageExports(addon.packageJSON, inAddonName),
specifier,
fromFile: addonConfig.canResolveFromFile,
fromPackageName: addon.name,
},
Expand Down
28 changes: 16 additions & 12 deletions packages/reverse-exports/src/index.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { posix } from 'path';
import { exports as resolveExports } from 'resolve.exports';

type PkgJSON = { name: string; exports?: Exports };
type Exports = string | string[] | { [key: string]: Exports };

/**
Expand Down Expand Up @@ -72,24 +73,27 @@ export function _findPathRecursively(
throw new Error(`Unexpected type of obj: ${typeof exportsObj}`);
}

export default function reversePackageExports(
{ exports: exportsObj, name }: { exports?: Exports; name: string },
relativePath: string
): string {
if (!exportsObj) {
return posix.join(name, relativePath);
/*
Takes a relativePath that is relative to the package root and produces its
externally-addressable name.
Returns undefined for a relativePath that is forbidden to be accessed from the
outside.
*/
export function externalName(pkg: PkgJSON, relativePath: string): string | undefined {
let { exports } = pkg;
if (!exports) {
return posix.join(pkg.name, relativePath);
}

const maybeKeyValuePair = _findPathRecursively(exportsObj, candidate => {
const maybeKeyValuePair = _findPathRecursively(exports, candidate => {
const regex = new RegExp(_prepareStringForRegex(candidate));

return regex.test(relativePath);
});

if (!maybeKeyValuePair) {
throw new Error(
`You tried to reverse exports for the file \`${relativePath}\` in package \`${name}\` but it does not match any of the exports rules defined in package.json. This means it should not be possible to access directly.`
);
return undefined;
}

const { key, value } = maybeKeyValuePair;
Expand All @@ -98,7 +102,7 @@ export default function reversePackageExports(
throw new Error('Expected value to be a string');
}

const maybeResolvedPaths = resolveExports({ name, exports: { [value]: key } }, relativePath);
const maybeResolvedPaths = resolveExports({ name: pkg.name, exports: { [value]: key } }, relativePath);

if (!maybeResolvedPaths) {
throw new Error(
Expand All @@ -108,7 +112,7 @@ export default function reversePackageExports(

const [resolvedPath] = maybeResolvedPaths;

return resolvedPath.replace(/^./, name);
return posix.join(pkg.name, resolvedPath);
}

export function _prepareStringForRegex(input: string): string {
Expand Down
56 changes: 27 additions & 29 deletions packages/reverse-exports/tests/reverse-exports.test.ts
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import reversePackageExports, { _findPathRecursively, _prepareStringForRegex } from '../src';
import { externalName, _findPathRecursively, _prepareStringForRegex } from '../src';

describe('reverse exports', function () {
it('exports is missing', function () {
expect(reversePackageExports({ name: 'best-addon' }, './dist/_app_/components/face.js')).toBe(
expect(externalName({ name: 'best-addon' }, './dist/_app_/components/face.js')).toBe(
'best-addon/dist/_app_/components/face.js'
);
});

it('exports is a string', function () {
const actual = reversePackageExports(
const actual = externalName(
{
name: 'my-addon',
exports: './foo.js',
Expand All @@ -19,7 +19,7 @@ describe('reverse exports', function () {
});

it('exports is an object with one entry', function () {
const actual = reversePackageExports(
const actual = externalName(
{
name: 'my-addon',
exports: {
Expand All @@ -43,12 +43,12 @@ describe('reverse exports', function () {
'./glob/*': './grod/**/*.js',
},
};
expect(reversePackageExports(packageJson, './main.js')).toBe('my-addon');
expect(reversePackageExports(packageJson, './secondary.js')).toBe('my-addon/sub/path');
expect(reversePackageExports(packageJson, './directory/some/file.js')).toBe('my-addon/prefix/some/file.js');
expect(reversePackageExports(packageJson, './other-directory/file.js')).toBe('my-addon/prefix/deep/file.js');
expect(reversePackageExports(packageJson, './yet-another/deep/file.js')).toBe('my-addon/other-prefix/deep/file');
expect(reversePackageExports(packageJson, './grod/very/deep/file.js')).toBe('my-addon/glob/very/deep/file');
expect(externalName(packageJson, './main.js')).toBe('my-addon');
expect(externalName(packageJson, './secondary.js')).toBe('my-addon/sub/path');
expect(externalName(packageJson, './directory/some/file.js')).toBe('my-addon/prefix/some/file.js');
expect(externalName(packageJson, './other-directory/file.js')).toBe('my-addon/prefix/deep/file.js');
expect(externalName(packageJson, './yet-another/deep/file.js')).toBe('my-addon/other-prefix/deep/file');
expect(externalName(packageJson, './grod/very/deep/file.js')).toBe('my-addon/glob/very/deep/file');
});

it('alternative exports', function () {
Expand All @@ -58,8 +58,8 @@ describe('reverse exports', function () {
'./things/': ['./good-things/', './bad-things/'],
},
};
expect(reversePackageExports(packageJson, './good-things/apple.js')).toBe('my-addon/things/apple.js');
expect(reversePackageExports(packageJson, './bad-things/apple.js')).toBe('my-addon/things/apple.js');
expect(externalName(packageJson, './good-things/apple.js')).toBe('my-addon/things/apple.js');
expect(externalName(packageJson, './bad-things/apple.js')).toBe('my-addon/things/apple.js');
});

it('conditional exports - simple abbreviated', function () {
Expand All @@ -71,9 +71,9 @@ describe('reverse exports', function () {
default: './index.js',
},
};
expect(reversePackageExports(packageJson, './index-module.js')).toBe('my-addon');
expect(reversePackageExports(packageJson, './index-require.cjs')).toBe('my-addon');
expect(reversePackageExports(packageJson, './index.js')).toBe('my-addon');
expect(externalName(packageJson, './index-module.js')).toBe('my-addon');
expect(externalName(packageJson, './index-require.cjs')).toBe('my-addon');
expect(externalName(packageJson, './index.js')).toBe('my-addon');
});

it('conditional exports - simple non-abbreviated', function () {
Expand All @@ -87,9 +87,9 @@ describe('reverse exports', function () {
},
},
};
expect(reversePackageExports(packageJson, './index-module.js')).toBe('my-addon');
expect(reversePackageExports(packageJson, './index-require.cjs')).toBe('my-addon');
expect(reversePackageExports(packageJson, './index.js')).toBe('my-addon');
expect(externalName(packageJson, './index-module.js')).toBe('my-addon');
expect(externalName(packageJson, './index-require.cjs')).toBe('my-addon');
expect(externalName(packageJson, './index.js')).toBe('my-addon');
});

it('conditional subpath exports', function () {
Expand All @@ -103,9 +103,9 @@ describe('reverse exports', function () {
},
},
};
expect(reversePackageExports(packageJson, './index.js')).toBe('my-addon');
expect(reversePackageExports(packageJson, './feature-node.cjs')).toBe('my-addon/feature.js');
expect(reversePackageExports(packageJson, './feature.js')).toBe('my-addon/feature.js');
expect(externalName(packageJson, './index.js')).toBe('my-addon');
expect(externalName(packageJson, './feature-node.cjs')).toBe('my-addon/feature.js');
expect(externalName(packageJson, './feature.js')).toBe('my-addon/feature.js');
});

it('nested conditional exports', function () {
Expand All @@ -119,12 +119,12 @@ describe('reverse exports', function () {
default: './feature.mjs',
},
};
expect(reversePackageExports(packageJson, './feature-node.mjs')).toBe('my-addon');
expect(reversePackageExports(packageJson, './feature-node.cjs')).toBe('my-addon');
expect(reversePackageExports(packageJson, './feature.mjs')).toBe('my-addon');
expect(externalName(packageJson, './feature-node.mjs')).toBe('my-addon');
expect(externalName(packageJson, './feature-node.cjs')).toBe('my-addon');
expect(externalName(packageJson, './feature.mjs')).toBe('my-addon');
});

it('should throw when no exports entry is matching', function () {
it('should return undefined when no exports entry is matching', function () {
const packageJson = {
name: 'my-addon',
exports: {
Expand All @@ -136,9 +136,7 @@ describe('reverse exports', function () {
},
};

expect(() => reversePackageExports(packageJson, './foo.bar')).toThrow(
'You tried to reverse exports for the file `./foo.bar` in package `my-addon` but it does not match any of the exports rules defined in package.json. This means it should not be possible to access directly.'
);
expect(externalName(packageJson, './foo.bar')).toBe(undefined);
});

it('conditional exports: using a single asterisk as glob for nested path', function () {
Expand All @@ -154,7 +152,7 @@ describe('reverse exports', function () {
},
};

expect(reversePackageExports(packageJson, './dist/_app_/components/welcome-page.js')).toBe(
expect(externalName(packageJson, './dist/_app_/components/welcome-page.js')).toBe(
'my-v2-addon/_app_/components/welcome-page'
);
});
Expand Down

0 comments on commit d445aff

Please sign in to comment.