jsonld.js icon indicating copy to clipboard operation
jsonld.js copied to clipboard

Add event handler

Open davidlehn opened this issue 5 years ago • 3 comments

  • An event handler option seemed like a good approach for a way to allow custom handling of warnings (to display in UIs or throw errors or whatever).
  • I was thinking a "strict mode" handler could be available that would turn warnings into errors.
  • The event approach with chainable handlers also turned out to work well for the case of capturing and replaying events for cached processed contexts.
  • I imagine other events beyond warnings might be useful.
  • Open to ideas for alternate approaches.

davidlehn avatar Feb 23 '20 02:02 davidlehn

Some rambling thoughts on this patch:

  • "handleEvent" params should be renamed "eventHandler".
  • I used "code" to align with error codes. Pondering how to best handle "code", "level", and maybe "type" concepts.
  • I started adding events for all the places where expansionMap/compactionMap skip something. Unsure how best to go about that as their functionality starts to overlap with events. The spec doesn't say dropping unknown data is a "warning", so I'm not sure what to call it to distinguish the two flows. "debug"? Maybe levels are a bad idea and it should be more like "defaultAction": "warning"? May need to fiddle with this to avoid too much output, yet be useful for debug tools.

davidlehn avatar Feb 23 '20 22:02 davidlehn

Some of the events happen in createTermDefinition. If event handlers were to be async, we'd have to make that async, and callers like _expandIri async, and maybe others up the stack. I was afraid that might have performance effects, but I didn't check. Another case where the in-progress benchmarking code needs to be improved so we can test such changes. I figured for version 1 the handlers could be sync and if we change to async/await in the future, they'll still work.

davidlehn avatar Feb 24 '20 17:02 davidlehn

Codecov Report

Merging #366 (1ebcc1f) into main (b7bc6d6) will increase coverage by 0.70%. The diff coverage is 94.44%.

@@            Coverage Diff             @@
##             main     #366      +/-   ##
==========================================
+ Coverage   92.67%   93.37%   +0.70%     
==========================================
  Files          23       24       +1     
  Lines        2880     2899      +19     
==========================================
+ Hits         2669     2707      +38     
+ Misses        211      192      -19     
Impacted Files Coverage Δ
lib/jsonld.js 83.17% <ø> (+0.32%) :arrow_up:
lib/events.js 89.28% <89.28%> (ø)
lib/context.js 94.56% <100.00%> (+3.25%) :arrow_up:
lib/expand.js 95.95% <100.00%> (+0.28%) :arrow_up:
lib/fromRdf.js 99.20% <100.00%> (+0.81%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b7bc6d6...1ebcc1f. Read the comment docs.

codecov-commenter avatar Mar 24 '22 04:03 codecov-commenter