[GitHub] [incubator-annotator] Treora commented on pull request #88: Restrict DOM scopes to be instances of Range

[GitBox <gi...@apache.org>]

Treora commented on pull request #88:
URL: https://github.com/apache/incubator-annotator/pull/88#issuecomment-688882247


   I hope you don’t mind I turned my suggestions (except using AbstractRange) into commits. I noticed that the `ownerDocument(range)` helper was not permitted by the eslint rule forbidding relative parent imports. Was that your reason to repeat the exact same lines in several files? I suggest we just remove this rule; even though it seems a nice idea, I think it’s really not worth obscuring our code with repeated rote implementation details just for the sake of organisational purity. Or, if you insist, we could publish our helper functions in a package; or (my preference) we’d modify the linter rule to allow relative parent imports from a single ‘utils’ or ‘common’ folder.
   
   It’s not directly related to this PR, but I also fixed two accidental uses of the global `document`, where `ownerDocument(range)` should have been used instead. Extra reason to keep this helper.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org