Merge pull request #26 from kennethnym/refactor/required-fetch-context

refactor: make fetchContext required on FeedSource
This commit is contained in:
2026-02-14 16:41:34 +00:00
committed by GitHub
12 changed files with 154 additions and 50 deletions

View File

@@ -74,7 +74,7 @@ function createWeatherSource(
async fetchContext(context) {
const location = contextValue(context, LocationKey)
if (!location) return {}
if (!location) return null
const weather = await fetchWeather(location)
return { [WeatherKey]: weather }
@@ -105,6 +105,10 @@ function createAlertSource(): FeedSource<AlertFeedItem> {
id: "alert",
dependencies: ["weather"],
async fetchContext() {
return null
},
async fetchItems(context) {
const weather = contextValue(context, WeatherKey)
if (!weather) return []
@@ -169,6 +173,9 @@ describe("FeedEngine", () => {
const orphan: FeedSource = {
id: "orphan",
dependencies: ["nonexistent"],
async fetchContext() {
return null
},
}
engine.register(orphan)
@@ -180,8 +187,20 @@ describe("FeedEngine", () => {
test("throws on circular dependency", () => {
const engine = new FeedEngine()
const a: FeedSource = { id: "a", dependencies: ["b"] }
const b: FeedSource = { id: "b", dependencies: ["a"] }
const a: FeedSource = {
id: "a",
dependencies: ["b"],
async fetchContext() {
return null
},
}
const b: FeedSource = {
id: "b",
dependencies: ["a"],
async fetchContext() {
return null
},
}
engine.register(a).register(b)
@@ -190,9 +209,27 @@ describe("FeedEngine", () => {
test("throws on longer cycles", () => {
const engine = new FeedEngine()
const a: FeedSource = { id: "a", dependencies: ["c"] }
const b: FeedSource = { id: "b", dependencies: ["a"] }
const c: FeedSource = { id: "c", dependencies: ["b"] }
const a: FeedSource = {
id: "a",
dependencies: ["c"],
async fetchContext() {
return null
},
}
const b: FeedSource = {
id: "b",
dependencies: ["a"],
async fetchContext() {
return null
},
}
const c: FeedSource = {
id: "c",
dependencies: ["b"],
async fetchContext() {
return null
},
}
engine.register(a).register(b).register(c)
@@ -282,7 +319,7 @@ describe("FeedEngine", () => {
const location: FeedSource = {
id: "location",
async fetchContext() {
return {} // No location available
return null // No location available
},
}
@@ -316,6 +353,9 @@ describe("FeedEngine", () => {
test("captures errors from fetchItems", async () => {
const failing: FeedSource = {
id: "failing",
async fetchContext() {
return null
},
async fetchItems() {
throw new Error("Items fetch failed")
},
@@ -340,6 +380,9 @@ describe("FeedEngine", () => {
const working: FeedSource = {
id: "working",
async fetchContext() {
return null
},
async fetchItems() {
return [
{

View File

@@ -89,10 +89,11 @@ export class FeedEngine<TItems extends FeedItem = FeedItem> {
// Run fetchContext in topological order
for (const source of graph.sorted) {
if (source.fetchContext) {
try {
const update = await source.fetchContext(context)
if (update) {
context = { ...context, ...update }
}
} catch (err) {
errors.push({
sourceId: source.id,
@@ -100,7 +101,6 @@ export class FeedEngine<TItems extends FeedItem = FeedItem> {
})
}
}
}
// Run fetchItems on all sources
const items: FeedItem[] = []
@@ -208,10 +208,12 @@ export class FeedEngine<TItems extends FeedItem = FeedItem> {
// Re-run fetchContext for dependents in order
for (const id of toRefresh) {
const source = graph.sources.get(id)
if (source?.fetchContext) {
if (source) {
try {
const update = await source.fetchContext(this.context)
if (update) {
this.context = { ...this.context, ...update }
}
} catch {
// Errors during reactive updates are logged but don't stop propagation
}

View File

@@ -73,7 +73,7 @@ function createWeatherSource(
async fetchContext(context) {
const location = contextValue(context, LocationKey)
if (!location) return {}
if (!location) return null
const weather = await fetchWeather(location)
return { [WeatherKey]: weather }
@@ -104,6 +104,10 @@ function createAlertSource(): FeedSource<AlertFeedItem> {
id: "alert",
dependencies: ["weather"],
async fetchContext() {
return null
},
async fetchItems(context) {
const weather = contextValue(context, WeatherKey)
if (!weather) return []
@@ -194,8 +198,8 @@ async function refreshGraph(graph: SourceGraph): Promise<{ context: Context; ite
// Run fetchContext in topological order
for (const source of graph.sorted) {
if (source.fetchContext) {
const update = await source.fetchContext(context)
if (update) {
context = { ...context, ...update }
}
}
@@ -245,9 +249,15 @@ describe("FeedSource", () => {
expect(source.id).toBe("alert")
expect(source.dependencies).toEqual(["weather"])
expect(source.fetchContext).toBeUndefined()
expect(source.fetchContext).toBeDefined()
expect(source.fetchItems).toBeDefined()
})
test("source without context returns null from fetchContext", async () => {
const source = createAlertSource()
const result = await source.fetchContext({ time: new Date() })
expect(result).toBeNull()
})
})
describe("graph validation", () => {
@@ -255,6 +265,9 @@ describe("FeedSource", () => {
const orphan: FeedSource = {
id: "orphan",
dependencies: ["nonexistent"],
async fetchContext() {
return null
},
}
expect(() => buildGraph([orphan])).toThrow(
@@ -263,16 +276,46 @@ describe("FeedSource", () => {
})
test("detects circular dependencies", () => {
const a: FeedSource = { id: "a", dependencies: ["b"] }
const b: FeedSource = { id: "b", dependencies: ["a"] }
const a: FeedSource = {
id: "a",
dependencies: ["b"],
async fetchContext() {
return null
},
}
const b: FeedSource = {
id: "b",
dependencies: ["a"],
async fetchContext() {
return null
},
}
expect(() => buildGraph([a, b])).toThrow("Circular dependency detected: a → b → a")
})
test("detects longer cycles", () => {
const a: FeedSource = { id: "a", dependencies: ["c"] }
const b: FeedSource = { id: "b", dependencies: ["a"] }
const c: FeedSource = { id: "c", dependencies: ["b"] }
const a: FeedSource = {
id: "a",
dependencies: ["c"],
async fetchContext() {
return null
},
}
const b: FeedSource = {
id: "b",
dependencies: ["a"],
async fetchContext() {
return null
},
}
const c: FeedSource = {
id: "c",
dependencies: ["b"],
async fetchContext() {
return null
},
}
expect(() => buildGraph([a, b, c])).toThrow("Circular dependency detected")
})
@@ -376,12 +419,12 @@ describe("FeedSource", () => {
})
test("source without location context returns empty items", async () => {
// Location source exists but hasn't been updated (returns default 0,0)
// Location source exists but hasn't been updated
const location: FeedSource = {
id: "location",
async fetchContext() {
// Simulate no location available
return {}
return null
},
}

View File

@@ -36,6 +36,13 @@ import type { FeedItem } from "./feed"
* return createWeatherFeedItems(ctx.weather)
* },
* }
*
* // TFL source - no context to provide
* const tflSource: FeedSource<TflFeedItem> = {
* id: "tfl",
* fetchContext: async () => null,
* fetchItems: async (ctx) => { ... },
* }
* ```
*/
export interface FeedSource<TItem extends FeedItem = FeedItem> {
@@ -58,8 +65,9 @@ export interface FeedSource<TItem extends FeedItem = FeedItem> {
/**
* Fetch context on-demand.
* Called during manual refresh or initial load.
* Return null if this source cannot provide context.
*/
fetchContext?(context: Context): Promise<Partial<Context>>
fetchContext(context: Context): Promise<Partial<Context> | null>
/**
* Subscribe to reactive feed item updates.

View File

@@ -58,7 +58,7 @@ export class CalendarSource implements FeedSource<CalendarFeedItem> {
this.injectedClient = options?.davClient ?? null
}
async fetchContext(context: Context): Promise<Partial<Context>> {
async fetchContext(context: Context): Promise<Partial<Context> | null> {
const events = await this.fetchEvents(context)
if (events.length === 0) {
return {

View File

@@ -197,13 +197,13 @@ describe("GoogleCalendarSource", () => {
})
describe("fetchContext", () => {
test("returns empty when no events", async () => {
test("returns null when no events", async () => {
const source = new GoogleCalendarSource({ client: createMockClient({ primary: [] }) })
const result = await source.fetchContext(createContext())
expect(result).toEqual({})
expect(result).toBeNull()
})
test("returns empty when only all-day events", async () => {
test("returns null when only all-day events", async () => {
const allDayOnly: ApiCalendarEvent[] = [
{
id: "evt-allday",
@@ -218,14 +218,15 @@ describe("GoogleCalendarSource", () => {
client: createMockClient({ primary: allDayOnly }),
})
const result = await source.fetchContext(createContext())
expect(result).toEqual({})
expect(result).toBeNull()
})
test("returns next upcoming timed event (not ongoing)", async () => {
const source = new GoogleCalendarSource({ client: defaultMockClient() })
const result = await source.fetchContext(createContext())
const nextEvent = contextValue(result as Context, NextEventKey)
expect(result).not.toBeNull()
const nextEvent = contextValue(result! as Context, NextEventKey)
expect(nextEvent).toBeDefined()
// evt-soon starts at 10:10, which is the nearest future timed event
expect(nextEvent!.title).toBe("1:1 with Manager")
@@ -250,7 +251,8 @@ describe("GoogleCalendarSource", () => {
})
const result = await source.fetchContext(createContext())
const nextEvent = contextValue(result as Context, NextEventKey)
expect(result).not.toBeNull()
const nextEvent = contextValue(result! as Context, NextEventKey)
expect(nextEvent).toBeDefined()
expect(nextEvent!.location).toBe("123 Main St")
})
@@ -270,7 +272,7 @@ describe("GoogleCalendarSource", () => {
client: createMockClient({ primary: events }),
})
const result = await source.fetchContext(createContext())
expect(result).toEqual({})
expect(result).toBeNull()
})
})

View File

@@ -75,14 +75,14 @@ export class GoogleCalendarSource implements FeedSource<CalendarFeedItem> {
this.lookaheadHours = options.lookaheadHours ?? DEFAULT_LOOKAHEAD_HOURS
}
async fetchContext(context: Context): Promise<Partial<Context>> {
async fetchContext(context: Context): Promise<Partial<Context> | null> {
const events = await this.fetchAllEvents(context.time)
const now = context.time.getTime()
const nextTimedEvent = events.find((e) => !e.isAllDay && e.startTime.getTime() > now)
if (!nextTimedEvent) {
return {}
return null
}
const minutesUntilStart = (nextTimedEvent.startTime.getTime() - now) / 60_000

View File

@@ -27,11 +27,11 @@ describe("LocationSource", () => {
expect(items).toEqual([])
})
test("fetchContext returns empty when no location", async () => {
test("fetchContext returns null when no location", async () => {
const source = new LocationSource()
const context = await source.fetchContext()
expect(context).toEqual({})
expect(context).toBeNull()
})
test("fetchContext returns location when available", async () => {

View File

@@ -73,11 +73,11 @@ export class LocationSource implements FeedSource {
}
}
async fetchContext(): Promise<Partial<Context>> {
async fetchContext(): Promise<Partial<Context> | null> {
if (this.lastLocation) {
return { [LocationKey]: this.lastLocation }
}
return {}
return null
}
async fetchItems(): Promise<[]> {

View File

@@ -77,6 +77,10 @@ export class TflSource implements FeedSource<TflAlertFeedItem> {
this.lines = options.lines ?? [...TflSource.DEFAULT_LINES_OF_INTEREST]
}
async fetchContext(): Promise<null> {
return null
}
/**
* Update the set of monitored lines. Takes effect on the next fetchItems call.
*/

View File

@@ -52,11 +52,11 @@ describe("WeatherSource", () => {
describe("fetchContext", () => {
const mockClient = createMockClient(fixture.response as WeatherKitResponse)
test("returns empty when no location", async () => {
test("returns null when no location", async () => {
const source = new WeatherSource({ client: mockClient })
const result = await source.fetchContext(createMockContext())
expect(result).toEqual({})
expect(result).toBeNull()
})
test("returns simplified weather context", async () => {
@@ -64,7 +64,8 @@ describe("WeatherSource", () => {
const context = createMockContext({ lat: 37.7749, lng: -122.4194 })
const result = await source.fetchContext(context)
const weather = contextValue(result, WeatherKey)
expect(result).not.toBeNull()
const weather = contextValue(result! as Context, WeatherKey)
expect(weather).toBeDefined()
expect(typeof weather!.temperature).toBe("number")
@@ -81,7 +82,8 @@ describe("WeatherSource", () => {
const context = createMockContext({ lat: 37.7749, lng: -122.4194 })
const result = await source.fetchContext(context)
const weather = contextValue(result, WeatherKey)
expect(result).not.toBeNull()
const weather = contextValue(result! as Context, WeatherKey)
// Fixture has temperature around 10°C, imperial should be around 50°F
expect(weather!.temperature).toBeGreaterThan(40)

View File

@@ -111,10 +111,10 @@ export class WeatherSource implements FeedSource<WeatherFeedItem> {
this.units = options.units ?? Units.metric
}
async fetchContext(context: Context): Promise<Partial<Context>> {
async fetchContext(context: Context): Promise<Partial<Context> | null> {
const location = contextValue(context, LocationKey)
if (!location) {
return {}
return null
}
const response = await this.client.fetch({
@@ -123,7 +123,7 @@ export class WeatherSource implements FeedSource<WeatherFeedItem> {
})
if (!response.currentWeather) {
return {}
return null
}
const weather: Weather = {