[x] We should pull shortDomains out of the runStudy function configuration object and, instead, import them through the LinkResolution module (e.g., call a function in the module for returning an array of domains, then pass that to a function in the Matching module to generate a regular expression).
[x] The resolution request handling in the browser.runtime.onMessage listener waits on all links from a content script request to resolve before sending a response that contains only the successful resolutions. We should revise this implementation in two ways.
[x] We should handle each link resolution request independently, since they’ll resolve at very different rates and the message passing overhead is (so far) low. I don’t have a preference on whether we allow batching link resolution requests (i.e., the content script can send an array of links to resolve, not just one link at a time).
[x] We should move the content script functionality into the LinkResolution module. It’s foreseeable that we’ll have other modules with content scripts that want to resolve links, just like it’s foreseeable that we’ll have other modules with background scripts that want to resolve links. I plan to eventually implement a similar approach for the PageEvents module, so that content scripts can subscribe to updates about page events.
[x] We’re resending the entire link exposure object on each tick, rather than just the changes to the object. That could blow up the size of message passing on pages with lots of links, and it’s often entirely redundant (e.g., if a page is static or not the active tab). We should try to only send over a delta to the object on each tick.
[x] There’s inconsistent use of camel case in the module. We should stick to that for consistency and readability.
[x] There’s inconsistent use of the debugging console. We should stick to one line of debugging output per event.
[x] Handle Google AMP URLs.
[x] Remove console.log.
[x] Handle shortened URLs that don't resolve successfully. For example, we might save them as link exposures with a boolean flag indicating that resolution failed.
Feedback from an initial code review:
shortDomains
out of therunStudy
function configuration object and, instead, import them through the LinkResolution module (e.g., call a function in the module for returning an array of domains, then pass that to a function in the Matching module to generate a regular expression).browser.runtime.onMessage
listener waits on all links from a content script request to resolve before sending a response that contains only the successful resolutions. We should revise this implementation in two ways.