Skip to content
Open
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
32 changes: 30 additions & 2 deletions packages/core/src/effect/layer-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,16 +78,35 @@ type MakeInput<
readonly tag?: T
}

/**
* Dependency arrays are built at module-evaluation time, so a circular import can leave an entry
* `undefined` instead of a node. That stays invisible until the graph is walked, where it surfaces
* as a `TypeError` reading `.name` off `undefined` with no indication of which module is at fault.
* Failing here instead names the node and the offending position.
*/
function checkDependencies(name: string, dependencies: readonly AnyNode[] | undefined) {
if (dependencies === undefined) return
const index = dependencies.findIndex((dependency) => dependency === undefined || dependency === null)
if (index === -1) return
throw new Error(
`Layer node "${name}" has an undefined dependency at index ${index}. ` +
`This usually means a circular import: the module that provides it has not finished ` +
`initializing. Break the cycle, for example by making the import type-only.`,
)
}

export function make<
const Implementation extends Layer.Any,
const Items extends NodeList,
const T extends Tag | undefined = undefined,
>(
input: MakeInput<Implementation, Items, T>,
): Node<Layer.Success<Implementation>, Layer.Error<Implementation> | Error<Items[number]>, T> {
const name = input.service !== undefined ? input.service.key : input.name
checkDependencies(name, input.deps)
return {
kind: "layer",
name: input.service !== undefined ? input.service.key : input.name,
name,
service: input.service,
implementation: input.layer,
dependencies: input.deps,
Expand All @@ -108,6 +127,7 @@ export function unbound<R, Shape, const T extends Tag>(service: Context.Key<R, S
export function group<const Items extends readonly AnyNode[]>(
dependencies: Items,
): Node<Output<Items[number]>, Error<Items[number]>, NodeTag<Items[number]>> {
checkDependencies("group", dependencies)
return { kind: "group", name: "group", dependencies }
}

Expand Down Expand Up @@ -193,6 +213,9 @@ function walk<Result>(
)
}

// validate before any visitor dereferences an entry
checkDependencies(target.name, target.dependencies)

visiting.add(target)
stack.push(target)
try {
Expand Down Expand Up @@ -303,6 +326,8 @@ function rewriteReplacementDependencies(root: AnyNode, replacements: ReadonlyMap
visiting.add(target)
stack.push(target)
try {
// this recursion is separate from `walk`, and hoisting a tagged subtree reaches it directly
checkDependencies(target.name, target.dependencies)
const dependencies = target.dependencies.map((dependency) => recur(dependency))
const result = dependencies.every((dependency, index) => dependency === target.dependencies[index])
? target
Expand All @@ -327,7 +352,10 @@ export function hasUnbound(root: Node<unknown, unknown, any>, source: AnyNode):
}

function flatten(node: AnyNode): readonly AnyNode[] {
return node.kind === "group" ? node.dependencies.flatMap(flatten) : [node]
if (node.kind !== "group") return [node]
// groups are expanded before the compile-time check runs, so validate here too
checkDependencies(node.name, node.dependencies)
return node.dependencies.flatMap(flatten)
}

export * as LayerNode from "./layer-node"
67 changes: 67 additions & 0 deletions packages/core/test/effect/layer-node/layer-node.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,73 @@ describe("layer node", () => {
expect(await Effect.runPromise(program)).toEqual(["first", "second"])
})

test("rejects an undefined dependency", () => {
// a circular import leaves the dependency undefined at module-evaluation time
// @ts-expect-error A dependency must be a node
expect(() => make({ service: Greeting, layer: greetingLayer, deps: [undefined] })).toThrow(
'Layer node "test/LayerNodeGreeting" has an undefined dependency at index 0',
)
})

test("reports the position of an undefined dependency", () => {
// @ts-expect-error A dependency must be a node
expect(() => make({ service: Greeting, layer: greetingLayer, deps: [value, undefined] })).toThrow(
"undefined dependency at index 1",
)
})

test("rejects an undefined dependency in a group", () => {
// @ts-expect-error A dependency must be a node
expect(() => LayerNode.group([value, undefined])).toThrow(
'Layer node "group" has an undefined dependency at index 1',
)
})

test("rejects an undefined dependency on a node that bypassed make", () => {
// `Node` is structural, so a node built by hand never passes through `make`
// @ts-expect-error A dependency must be a node
const handBuilt: LayerNode.Node<Greeting> = { ...greeting, dependencies: [undefined] }
expect(() => LayerNode.compile(handBuilt)).toThrow(
'Layer node "test/LayerNodeGreeting" has an undefined dependency at index 0',
)
})

// groups are expanded by `flatten` before a node is visited, so they need their own coverage
const handBuiltGroup = (): LayerNode.Node<Value> => ({
kind: "group",
name: "external-group",
// @ts-expect-error A dependency must be a node
dependencies: [undefined],
})

test("rejects an undefined dependency in a hand-built group at the root", () => {
expect(() => LayerNode.compile(handBuiltGroup())).toThrow(
'Layer node "external-group" has an undefined dependency at index 0',
)
})

test("rejects an undefined dependency in a nested hand-built group", () => {
const parent = make({ service: Greeting, layer: greetingLayer, deps: [handBuiltGroup()] })
expect(() => LayerNode.compile(parent)).toThrow(
'Layer node "external-group" has an undefined dependency at index 0',
)
})

test("rejects an undefined dependency while hoisting", () => {
expect(() => LayerNode.hoist(handBuiltGroup(), tags.values.app)).toThrow(
'Layer node "external-group" has an undefined dependency at index 0',
)
})

test("rejects an undefined dependency while rewriting replacements", () => {
// a hoisted tagged node is rewritten rather than visited, so it reaches a separate recursion
const parent = make({ service: Greeting, layer: greetingLayer, deps: [handBuiltGroup()] })
const replacement = Layer.succeed(Value, Value.of({ value: "simulation" }))
expect(() => LayerNode.hoist(parent, tags.values.app, [[value, replacement]])).toThrow(
'Layer node "external-group" has an undefined dependency at index 0',
)
})

test("requires unbound nodes to be replaced before compilation", async () => {
const unbound = LayerNode.unbound(Value, tags.values.app)
const greeting = make({ service: Greeting, layer: greetingLayer, deps: [unbound] })
Expand Down
Loading