Skip to content

Commit

Permalink
Improve error message when metadata is missing (fixes #55)
Browse files Browse the repository at this point in the history
  • Loading branch information
mbeckem committed Jun 11, 2024
1 parent bfc51aa commit d6d9c72
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 18 deletions.
1 change: 1 addition & 0 deletions src/packages/runtime/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export enum ErrorId {
INTERFACE_NOT_FOUND = "runtime:interface-not-found",
AMBIGUOUS_DEPENDENCY = "runtime:ambiguous-dependency",
UNDECLARED_DEPENDENCY = "runtime:undeclared-dependency",
MISSING_PACKAGE = "runtime:missing-package",
SERVICE_CONSTRUCTION_FAILED = "runtime:service-construction-failed",
SERVICE_DESTRUCTION_FAILED = "runtime:service-destruction-failed",
DUPLICATE_INTERFACE = "runtime:duplicate-interface",
Expand Down
29 changes: 25 additions & 4 deletions src/packages/runtime/react-integration/ReactIntegration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,35 @@ it("should get error when using undefined service", async () => {
errorSpy.mockImplementation(doNothing);

function TestComponent() {
const service = useServiceInternal<unknown>("test", "test.Provider") as TestProvider;
const service = useServiceInternal<unknown>(
"test",
"test.InterfaceThatDoesNotExist"
) as TestProvider;
return createElement("span", undefined, `Hello ${service.value}`);
}

const { integration } = createIntegration({
disablePackage: true
});
const { integration } = createIntegration();

expect(() => {
act(() => {
integration.render(TestComponent);
});
}).toThrowErrorMatchingSnapshot();
expect(errorSpy).toHaveBeenCalledOnce();
});

it("reports a helpful error when package metadata is missing", async () => {
// This problem can happen when a package is not correctly declared as a dependency in package.json
// (so its metadata are not discovered by the vite plugin) but it is still imported from the application
// where it attempts to use a service.
errorSpy.mockImplementation(doNothing);

function TestComponent() {
const service = useServiceInternal<unknown>("packageMissingFromMetadata", "test.Provider");
return String(service);
}

const { integration } = createIntegration();
expect(() => {
act(() => {
integration.render(TestComponent);
Expand Down
17 changes: 17 additions & 0 deletions src/packages/runtime/react-integration/ReactIntegration.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ export class ReactIntegration {
` Possible choices are: ${renderedChoices}.`
);
}
case "unknown-package":
throw new Error(
ErrorId.MISSING_PACKAGE,
missingPackageMessage(packageName, interfaceName)
);
}
},
getServices: (packageName, interfaceName) => {
Expand All @@ -78,6 +83,11 @@ export class ReactIntegration {
`Package '${packageName}' did not declare an UI dependency on all services implementing interface '${interfaceName}'.` +
` Add the dependency ("all": true) to the package configuration or remove the usage.`
);
case "unknown-package":
throw new Error(
ErrorId.MISSING_PACKAGE,
missingPackageMessage(packageName, interfaceName)
);
}
},
getProperties: (packageName) => {
Expand Down Expand Up @@ -120,3 +130,10 @@ export class ReactIntegration {
return pkg;
}
}

function missingPackageMessage(packageName: string, interfaceName: string) {
return (
`Package '${packageName}' was not found in the application's metadata while it attempted to reference the interface '${interfaceName}'. ` +
`Check that the dependency is declared correctly in the packages that use '${packageName}'.`
);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html

exports[`reports a helpful error when package metadata is missing 1`] = `[Error: runtime:missing-package: Package 'packageMissingFromMetadata' was not found in the application's metadata while it attempted to reference the interface 'test.Provider'. Check that the dependency is declared correctly in the packages that use 'packageMissingFromMetadata'.]`;

exports[`should allow access to all services via react hook 1`] = `
<span>
Joined Values: TEST1,TEST2,TEST3
Expand Down Expand Up @@ -28,7 +30,7 @@ exports[`should deny access to all services if declaration is missing 1`] = `[Er

exports[`should deny access to service when the qualifier does not match 1`] = `[Error: runtime:undeclared-dependency: Package 'test' did not declare an UI dependency on interface 'test.Provider' (qualifier: 'bar'). Add the dependency to the package configuration or remove the usage.]`;

exports[`should get error when using undefined service 1`] = `[Error: runtime:undeclared-dependency: Package 'test' did not declare an UI dependency on interface 'test.Provider'. Add the dependency to the package configuration or remove the usage.]`;
exports[`should get error when using undefined service 1`] = `[Error: runtime:undeclared-dependency: Package 'test' did not declare an UI dependency on interface 'test.InterfaceThatDoesNotExist'. Add the dependency to the package configuration or remove the usage.]`;

exports[`should provide the autogenerated useProperties hook 1`] = `
<div
Expand Down
49 changes: 36 additions & 13 deletions src/packages/runtime/service-layer/ServiceLayer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,23 @@ const LOG = createLogger("runtime:ServiceLayer");

export type DynamicLookupResult = ServiceLookupResult | UndeclaredDependency;

/**
* Returned when a dependency is not declared in the package's metadata.
*/
export interface UndeclaredDependency {
type: "undeclared";
}

/**
* Returned when a package is not found in the application's metadata.
*/
export interface UnknownPackage {
type: "unknown-package";
}

const UNDECLARED_DEPENDENCY = { type: "undeclared" } satisfies UndeclaredDependency;
const UNKNOWN_PACKAGE = { type: "unknown-package" } satisfies UnknownPackage;

interface DependencyDeclarations {
all: boolean;
unqualified: boolean;
Expand Down Expand Up @@ -139,13 +152,16 @@ export class ServiceLayer {
packageName: string,
spec: InterfaceSpec,
options?: { ignoreDeclarationCheck?: boolean }
): ServiceLookupResult | UndeclaredDependency {
): ServiceLookupResult | UndeclaredDependency | UnknownPackage {
if (this.state !== "started") {
throw new Error(ErrorId.INTERNAL, "Service layer is not started.");
}

if (!options?.ignoreDeclarationCheck && !this.isDeclaredDependency(packageName, spec)) {
return { type: "undeclared" };
if (!options?.ignoreDeclarationCheck) {
const checkError = this.checkDependency(packageName, spec);
if (checkError) {
return checkError;
}
}

return this.serviceLookup.lookupOne(spec);
Expand All @@ -163,15 +179,15 @@ export class ServiceLayer {
getServices(
packageName: string,
interfaceName: string
): ServicesLookupResult | UndeclaredDependency {
): ServicesLookupResult | UndeclaredDependency | UnknownPackage {
if (this.state !== "started") {
throw new Error(ErrorId.INTERNAL, "Service layer is not started.");
}

if (!this.isDeclaredDependency(packageName, { interfaceName, all: true })) {
return { type: "undeclared" };
const checkError = this.checkDependency(packageName, { interfaceName, all: true });
if (checkError) {
return checkError;
}

return this.serviceLookup.lookupAll(interfaceName);
}

Expand Down Expand Up @@ -238,23 +254,30 @@ export class ServiceLayer {
}
}

private isDeclaredDependency(packageName: string, spec: ReferenceSpec) {
/** Undefined -> everything is okay */
private checkDependency(
packageName: string,
spec: ReferenceSpec
): UnknownPackage | UndeclaredDependency | undefined {
const packageEntry = this.declaredDependencies.get(packageName);
if (!packageEntry) {
return false;
return UNKNOWN_PACKAGE;
}

const interfaceEntry = packageEntry.get(spec.interfaceName);
if (!interfaceEntry) {
return false;
return UNDECLARED_DEPENDENCY;
}

if (isSingleImplementationSpec(spec)) {
if (spec.qualifier == null) {
return interfaceEntry.unqualified;
return interfaceEntry.unqualified ? undefined : UNDECLARED_DEPENDENCY;
}
return interfaceEntry.qualifiers.has(spec.qualifier);
return interfaceEntry.qualifiers.has(spec.qualifier)
? undefined
: UNDECLARED_DEPENDENCY;
} else {
return interfaceEntry.all;
return interfaceEntry.all ? undefined : UNDECLARED_DEPENDENCY;
}
}

Expand Down

0 comments on commit d6d9c72

Please sign in to comment.