Add feature for setting docs & protocolHandlers from constructor.

[aljones15]

Adds a new minor feature that allows you to pass in a documents Map and a protocolHandler map to the constructor avoiding the loop of using addStatic and setProtocolHandler.
This makes creation of multiple documentLoaders much easier.
Also adds 2 tests to the project that use this constructor.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.49%. Comparing base (9ef1abd) to head (1963d17).

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #11      +/-   ##
==========================================
+ Coverage   98.27%   98.49%   +0.22%     
==========================================
  Files           2        2              
  Lines         116      133      +17     
==========================================
+ Hits          114      131      +17     
  Misses          2        2              

Files	Coverage Δ
lib/JsonLdDocumentLoader.js	98.43% <100.00%> (+0.23%)	⬆️

---

Continue to review full report in Codecov by Sentry.

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

[davidlehn]

• Please add some docs/examples to the README for this feature.
• It might also be worth noting these are mutable params and direct changes or changes via the other API calls will effect every loader using the Maps.
• I do have a concern that this is exposing internal implementation details as an API. I'm not sure what the future ramifications of that might be, if any.

[dlongley]

I do have a concern that this is exposing internal implementation details as an API. I'm not sure what the future ramifications of that might be, if any.

Yeah, I agree with this. Perhaps it would be better to allow addStatic to take a Map of documents that will be processed and added to the internal implementation instead of passing and using something directly in the constructor. It could get weird with the map being used by reference and potentially updated later as well. I don't know that we have the same kind of use case for setProtocolHandler where adding the ability to accept a Map adds much, so we could probably skip that -- unless I'm missing something there.

[aljones15]

I do have a concern that this is exposing internal implementation details as an API. I'm not sure what the future ramifications of that might be, if any.

Yeah, I agree with this. Perhaps it would be better to allow addStatic to take a Map of documents that will be processed and added to the internal implementation instead of passing and using something directly in the constructor. It could get weird with the map being used by reference and potentially updated later as well. I don't know that we have the same kind of use case for setProtocolHandler where adding the ability to accept a Map adds much, so we could probably skip that -- unless I'm missing something there.

you do have a point, the Map is by reference and could be mutated elsewhere. Perhaps we could simply copy the Maps passed in? As protocolHandlers it's set in the constructor so I just included it there.

did this here: 7a4bf3e

Example:

const mapOne = new Map([['one', 1]]);
const mapTwo = new Map(mapOne);
mapOne.set('one', 2);
mapOne.get('one');
// returns 2
mapTwo.get('one');
// returns 1

I can add a test to ensure mutations to a Map passed into the documentLoader do not mutate entries in documents if wanted.

[aljones15]

@davidlehn @dlongley also of note: the existing addStatic function copies the document by reference into this.documents which means mutations to the original document will show up in this.documents so maybe we need to clone documents on add?

[aljones15]

closing as this does not appear to be the direction this library wants to go in.