You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
parseICS currently does two things depending on whether you pass a callback:
constdata=ical.parseICS(str);// sync — returns resultical.parseICS(str,(err,data)=>{…});// async — callback or Promise
This was introduced in d492338 (2019) as a migration bridge from callbacks to Promises. It served its purpose, but the dual-mode design has real downsides:
Confusing to read. Nothing in the name says "this can be async". You have to check the call site to know what it does.
Hard to type. The return type is CalendarResponse | undefined depending on the arguments. TypeScript can only express this with awkward overloads.
Internal complexity. Supporting both modes requires a promiseCallback bridge utility and a forked code path in ical.js — complexity that exists solely to hold this API together.
I'd like to split this into two explicit functions. Would appreciate feedback on the approach.
Proposed API
Function
Contract
parseICS(str)
Sync. Returns result. Throws on error.
parseICSAsync(str)
Returns Promise<CalendarResponse>.
Rollout
Step 1 — non-breaking:
Export parseICSAsync(str) as a new async function
When parseICS is called with a callback, emit a deprecation warning
Callers already using parseICS(str) without a callback need no changes.
Internal simplification
The split also cleans up a meaningful amount of internal complexity:
promiseCallback goes away. This ~35-line utility exists solely to bridge the dual-mode API — wrapping a Promise so it can optionally call a callback, with extra bookkeeping (hasResult/callbackError) to prevent double-invocations if the user's callback throws. Without the dual-mode, it has no job.
ical.jsparseICS loses its fork. The current if (cb) { setImmediate(…) } else { return … } becomes two lean, single-purpose functions.
fromURL and parseFile collapse. Both currently thread a callback through a promiseCallback → ical.parseICS(data, cb) sandwich. After the split: const data = await fetch(…); return ical.parseICSAsync(data); — three lines, no ceremony.
setImmediate recursion → async/await for-loop. The current async path uses recursive setImmediate() callbacks to batch line processing, which fragments stack traces and makes errors hard to catch (Uncatachable exception "No toISOString function in exdate[name]" #144). With a proper async function this becomes a plain for loop that awaits a setImmediate-based yield every N lines — same event-loop friendliness, linear control flow, and natural error propagation via try/catch.
Future: AbortController support
Once parseICSAsync is a real async function, adding cancellation becomes trivial:
This would allow timeouts on large files or user-initiated cancellation — a non-breaking addition (optional parameter) that fits naturally once the async path is clean. Not part of this proposal, but it becomes possible.
Alternatives
Option
Trade-off
A
Do nothing
The problems above remain. The fix for #144 patched the symptom, not the design.
B
All async
Sync parsing is CPU-only, legitimate for scripts/startup. Bigger break for no gain.
C
All sync
Loses setImmediate batching needed for large calendars in server contexts.
D
parseICS(str, { async: true })
Same problem in different clothes. Types stay ugly.
E
Dedicated parseICSAsync
Clean split. Both use cases preserved. No break for sync callers.
parseICScurrently does two things depending on whether you pass a callback:This was introduced in d492338 (2019) as a migration bridge from callbacks to Promises. It served its purpose, but the dual-mode design has real downsides:
CalendarResponse | undefineddepending on the arguments. TypeScript can only express this with awkward overloads.promiseCallbackbridge utility and a forked code path inical.js— complexity that exists solely to hold this API together.I'd like to split this into two explicit functions. Would appreciate feedback on the approach.
Proposed API
parseICS(str)parseICSAsync(str)Promise<CalendarResponse>.Rollout
Step 1 — non-breaking:
parseICSAsync(str)as a new async functionparseICSis called with a callback, emit a deprecation warningStep 2 — breaking:
parseICSMigration
Callers already using
parseICS(str)without a callback need no changes.Internal simplification
The split also cleans up a meaningful amount of internal complexity:
promiseCallbackgoes away. This ~35-line utility exists solely to bridge the dual-mode API — wrapping a Promise so it can optionally call a callback, with extra bookkeeping (hasResult/callbackError) to prevent double-invocations if the user's callback throws. Without the dual-mode, it has no job.ical.jsparseICSloses its fork. The currentif (cb) { setImmediate(…) } else { return … }becomes two lean, single-purpose functions.fromURLandparseFilecollapse. Both currently thread a callback through apromiseCallback → ical.parseICS(data, cb)sandwich. After the split:const data = await fetch(…); return ical.parseICSAsync(data);— three lines, no ceremony.setImmediaterecursion →async/awaitfor-loop. The current async path uses recursivesetImmediate()callbacks to batch line processing, which fragments stack traces and makes errors hard to catch (Uncatachable exception "No toISOString function in exdate[name]" #144). With a proper async function this becomes a plainforloop thatawaits asetImmediate-based yield every N lines — same event-loop friendliness, linear control flow, and natural error propagation viatry/catch.Future:
AbortControllersupportOnce
parseICSAsyncis a realasyncfunction, adding cancellation becomes trivial:This would allow timeouts on large files or user-initiated cancellation — a non-breaking addition (optional parameter) that fits naturally once the async path is clean. Not part of this proposal, but it becomes possible.
Alternatives
setImmediatebatching needed for large calendars in server contexts.parseICS(str, { async: true })parseICSAsyncContext