Skip to content

Commit

Permalink
Implemented improved behavior of #633
Browse files Browse the repository at this point in the history
  • Loading branch information
FlorianRappl committed Oct 16, 2023
1 parent a94053a commit be47a24
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 13 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Piral Changelog

## 1.3.1 (tbd)

- Updated behavior with unresolved inherited dependencies (#633)

## 1.3.0 (October 9, 2023)

- Fixed issue with global installation in pnpm (#624)
Expand Down
86 changes: 85 additions & 1 deletion src/tooling/piral-cli/src/common/importmap.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@ import { readImportmap } from './importmap';

jest.mock('./npm', () => ({
tryResolvePackage(id, dir) {
return `${dir}/${id}/index.js`;
if (id === 'qxz') {
return undefined;
} else if (id.endsWith('package.json')) {
return `${dir}/${id}`;
} else {
return `${dir}/${id}/index.js`;
}
},
}));

Expand All @@ -15,6 +21,28 @@ const mockPackages = {
name: 'bar',
version: '1.0.0',
},
'/data/app1': {
name: 'app1',
version: '1.0.0',
importmap: {
imports: {
'[email protected]': 'foo',
},
},
},
'/data/app1/foo': {
name: 'foo',
version: '1.2.3',
},
'/data/app2': {
name: 'app1',
version: '1.0.0',
importmap: {
imports: {
'[email protected]': '',
},
},
},
};

jest.mock('./io', () => ({
Expand Down Expand Up @@ -191,4 +219,60 @@ describe('Importmap', () => {
),
).rejects.toThrow();
});

it('fails when the explicitly shared package is not installed', async () => {
await expect(
readImportmap(
'/data',
{
importmap: {
imports: {
'qxz': '',
},
},
},
false,
),
).rejects.toThrow();
});

it('accepts an importmap with valid resolvable inherited deps', async () => {
const deps = await readImportmap(
'/data',
{
importmap: {
imports: {},
inherit: ['app1'],
},
},
false,
);
expect(deps).toEqual([
{
alias: undefined,
entry: '/data/app1/foo/index.js',
id: '[email protected]',
isAsync: false,
name: 'foo',
ref: 'foo.js',
requireId: '[email protected]',
type: 'local',
parents: ['app1'],
},
]);
});

it('accepts an importmap with valid but unresolvable inherited deps', async () => {
const deps = await readImportmap(
'/data',
{
importmap: {
imports: {},
inherit: ['app2'],
},
},
false,
);
expect(deps).toEqual([]);
});
});
49 changes: 37 additions & 12 deletions src/tooling/piral-cli/src/common/importmap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,25 +95,44 @@ async function getInheritedDependencies(inheritedImport: string, dir: string): P
if (packageJson) {
const packageDir = dirname(packageJson);
const packageDetails = await readJson(packageDir, 'package.json');
return readImportmap(packageDir, packageDetails, true, 'exact');
return await readImportmap(packageDir, packageDetails, true, 'exact');
} else {
const directImportmap = tryResolvePackage(inheritedImport, dir);

if (directImportmap) {
const baseDir = dirname(directImportmap);
const content = await readJson(baseDir, basename(directImportmap));
return await resolveImportmap(baseDir, content, 'exact');
return await resolveImportmap(baseDir, content, {
ignoreFailure: true,
versionBehavior: 'exact',
});
}
}

return [];
}

async function resolveImportmap(dir: string, importmap: Importmap, versionBehavior: ImportmapVersions): Promise<Array<SharedDependency>> {
interface ImportmapResolutionOptions {
versionBehavior: ImportmapVersions;
ignoreFailure: boolean;
}

async function resolveImportmap(
dir: string,
importmap: Importmap,
options: ImportmapResolutionOptions,
): Promise<Array<SharedDependency>> {
const dependencies: Array<SharedDependency> = [];
const sharedImports = importmap?.imports;
const inheritedImports = importmap?.inherit;
const excludedImports = importmap?.exclude;
const onUnresolved = (entry: string) => {
if (options.ignoreFailure) {
log('skipUnresolvedDependency_0054', entry);
} else {
fail('importMapReferenceNotFound_0027', dir, entry);
}
};

if (typeof sharedImports === 'object' && sharedImports) {
for (const depName of Object.keys(sharedImports)) {
Expand Down Expand Up @@ -143,7 +162,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
packageJson,
depName,
versionSpec,
versionBehavior,
options.versionBehavior,
);

addLocalDependencies(
Expand All @@ -157,7 +176,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
isAsync,
);
} else {
fail('importMapReferenceNotFound_0027', dir, identifier);
onUnresolved(identifier);
}
} else if (!url.startsWith('.') && !isAbsolute(url)) {
const entry = tryResolvePackage(url, dir);
Expand All @@ -168,7 +187,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
packageJson,
depName,
versionSpec,
versionBehavior,
options.versionBehavior,
);

addLocalDependencies(
Expand All @@ -182,7 +201,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
isAsync,
);
} else {
fail('importMapReferenceNotFound_0027', dir, url);
onUnresolved(url);
}
} else {
const entry = resolve(dir, url);
Expand All @@ -200,7 +219,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
packageJson,
depName,
versionSpec,
versionBehavior,
options.versionBehavior,
);

addLocalDependencies(
Expand All @@ -214,7 +233,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
isAsync,
);
} else if (isDirectory) {
fail('importMapReferenceNotFound_0027', entry, 'package.json');
onUnresolved(entry);
} else {
const hash = await getHash(entry);

Expand All @@ -229,7 +248,7 @@ async function resolveImportmap(dir: string, importmap: Importmap, versionBehavi
});
}
} else {
fail('importMapReferenceNotFound_0027', dir, url);
onUnresolved(url);
}
}
}
Expand Down Expand Up @@ -278,7 +297,10 @@ export async function readImportmap(
}

const baseDir = dirname(resolve(dir, importmap));
return await resolveImportmap(baseDir, content, versionBehavior);
return await resolveImportmap(baseDir, content, {
ignoreFailure: inherited,
versionBehavior,
});
} else if (typeof importmap === 'undefined' && inherited) {
// Fall back to sharedDependencies or pilets.external if available
const shared: Array<string> = packageDetails.sharedDependencies ?? packageDetails.pilets?.externals;
Expand All @@ -295,5 +317,8 @@ export async function readImportmap(
}
}

return await resolveImportmap(dir, importmap, versionBehavior);
return await resolveImportmap(dir, importmap, {
ignoreFailure: inherited,
versionBehavior,
});
}
22 changes: 22 additions & 0 deletions src/tooling/piral-cli/src/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,28 @@ export function cannotResolveDependency_0053(names: Array<string>, rootDir: stri
];
}

/**
* @kind Info
*
* @summary
* Reported when an inherited dependency cannot be resolved.
*
* @abstract
* When a pilet is built all the inherited (i.e., centrally shared) dependencies will be resolved. In case
* you did not install one of these dependencies a short info will be shown. This acts as a reminder that
* you could install more dependencies - without any runtime cost.
*
* Note that even though shared dependencies are available at runtime in any case they will might be
* for building your pilet. Therefore, if you plan to use shared dependencies please install them in your
* pilet's repository.
*
* @see
* - [Piral Instance Package Definition](https://docs.piral.io/reference/documentation/C21-piral-metadata)
*/
export function skipUnresolvedDependency_0054(name: string): QuickMessage {
return [LogLevels.info, '0054', `The inherited dependency "${name}" is not installed and will be skipped.`];
}

/**
* @kind Error
*
Expand Down

0 comments on commit be47a24

Please sign in to comment.