Eliminate extraneous interfaces
Make several changes to eliminate unnecessary type definitions.
Remove the centrally-exported selector interfaces from the selector
package and migrate them to where they are used. The selector package
need not commit to the existence of any particular set of selectors. In
the future, the apache-annotator package may consolidate the existing
selector definitions and export them under a unified namespace.
Remove the matcher interface in favor of being explicit and verbose
about function types.
Simplify the type signature for makeRefinable. The selector is defined
as an interface, thus callers are free to pass anything that extends it,
so there is no need to parametrize its type. Use the same type
definition for the input and output of the selector creator.
Define the DOM match functions in terms of Range and place the burden on
the caller to pass a Range. Remove the unused scope functions. This
change may hurt the developer experience, but that will require some
experience and feedback.
diff --git a/packages/dom/package.json b/packages/dom/package.json
index e2b198e..6704d56 100644
--- a/packages/dom/package.json
+++ b/packages/dom/package.json
@@ -22,9 +22,6 @@
"core-js": "^3.6.4",
"dom-seek": "^5.1.0"
},
- "devDependencies": {
- "@annotator/selector": "^0.1.0"
- },
"engines": {
"node": "^10 || ^11 || ^12 || >=13.7"
},
diff --git a/packages/dom/src/css.ts b/packages/dom/src/css.ts
index 28ffb94..a8c480a 100644
--- a/packages/dom/src/css.ts
+++ b/packages/dom/src/css.ts
@@ -18,12 +18,15 @@
* under the License.
*/
-import type { CssSelector, Matcher } from '@annotator/selector';
+export interface CssSelector {
+ type: 'CssSelector';
+ value: string;
+}
export function createCssSelectorMatcher(
selector: CssSelector,
-): Matcher<Document, Element> {
- return async function* matchAll(scope: Document) {
+): (scope: ParentNode) => AsyncIterable<Element> {
+ return async function* matchAll(scope) {
yield* scope.querySelectorAll(selector.value);
};
}
diff --git a/packages/dom/src/highlight-range.ts b/packages/dom/src/highlight-range.ts
index e7b9b8e..c4928c9 100644
--- a/packages/dom/src/highlight-range.ts
+++ b/packages/dom/src/highlight-range.ts
@@ -74,7 +74,8 @@
// Collect the text nodes.
const document =
- range.startContainer.ownerDocument || (range.startContainer as Document);
+ range.commonAncestorContainer.ownerDocument ??
+ (range.commonAncestorContainer as Document);
const walker = document.createTreeWalker(
range.commonAncestorContainer,
NodeFilter.SHOW_TEXT,
diff --git a/packages/dom/src/range/match.ts b/packages/dom/src/range/match.ts
index df87db7..bc2d601 100644
--- a/packages/dom/src/range/match.ts
+++ b/packages/dom/src/range/match.ts
@@ -18,22 +18,26 @@
* under the License.
*/
-import type { RangeSelector, Selector } from '@annotator/selector';
-
-import { ownerDocument } from '../scope';
-import type { DomMatcher, DomScope } from '../types';
-
import { product } from './cartesian';
-export function makeCreateRangeSelectorMatcher(
- createMatcher: <T extends Selector>(selector: T) => DomMatcher,
-): (selector: RangeSelector) => DomMatcher {
- return function createRangeSelectorMatcher(selector: RangeSelector) {
+export interface RangeSelector<T> {
+ type: 'RangeSelector';
+ startSelector: T;
+ endSelector: T;
+}
+
+export function makeCreateRangeSelectorMatcher<T>(
+ createMatcher: (selector: T) => (scope: Range) => AsyncIterable<Range>,
+): (selector: RangeSelector<T>) => (scope: Range) => AsyncIterable<Range> {
+ return function createRangeSelectorMatcher(selector) {
const startMatcher = createMatcher(selector.startSelector);
const endMatcher = createMatcher(selector.endSelector);
- return async function* matchAll(scope: DomScope) {
- const document = ownerDocument(scope);
+ return async function* matchAll(scope: Range) {
+ const { commonAncestorContainer } = scope;
+ const document =
+ commonAncestorContainer.ownerDocument ??
+ (commonAncestorContainer as Document);
const startMatches = startMatcher(scope);
const endMatches = endMatcher(scope);
diff --git a/packages/dom/src/scope.ts b/packages/dom/src/scope.ts
deleted file mode 100644
index e107212..0000000
--- a/packages/dom/src/scope.ts
+++ /dev/null
@@ -1,39 +0,0 @@
-/**
- * @license
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import type { DomScope } from './types';
-
-export function ownerDocument(scope: DomScope): Document {
- const node = isRange(scope) ? scope.commonAncestorContainer : scope;
- return node.ownerDocument || (node as Document);
-}
-
-export function rangeFromScope(scope: DomScope): Range {
- if (isRange(scope)) {
- return scope;
- }
- const range = ownerDocument(scope).createRange();
- range.selectNodeContents(scope);
- return range;
-}
-
-function isRange(scope: DomScope): scope is Range {
- return 'collapsed' in scope;
-}
diff --git a/packages/dom/src/text-quote/describe.ts b/packages/dom/src/text-quote/describe.ts
index b048914..bfa1ea3 100644
--- a/packages/dom/src/text-quote/describe.ts
+++ b/packages/dom/src/text-quote/describe.ts
@@ -19,46 +19,34 @@
*/
import seek from 'dom-seek';
-import type { TextQuoteSelector } from '@annotator/selector';
-import type { DomScope } from '../types';
-import { ownerDocument, rangeFromScope } from '../scope';
+import type { TextQuoteSelector } from './selector';
export async function describeTextQuote(
range: Range,
- scope: DomScope = ownerDocument(range).documentElement,
+ scope: Range = range,
): Promise<TextQuoteSelector> {
range = range.cloneRange();
// Take the part of the range that falls within the scope.
- const scopeAsRange = rangeFromScope(scope);
- if (!scopeAsRange.isPointInRange(range.startContainer, range.startOffset))
- range.setStart(scopeAsRange.startContainer, scopeAsRange.startOffset);
- if (!scopeAsRange.isPointInRange(range.endContainer, range.endOffset))
- range.setEnd(scopeAsRange.endContainer, scopeAsRange.endOffset);
+ if (!scope.isPointInRange(range.startContainer, range.startOffset))
+ range.setStart(scope.startContainer, scope.startOffset);
+ if (!scope.isPointInRange(range.endContainer, range.endOffset))
+ range.setEnd(scope.endContainer, scope.endOffset);
- const exact = range.toString();
-
- const result: TextQuoteSelector = { type: 'TextQuoteSelector', exact };
-
- const { prefix, suffix } = calculateContextForDisambiguation(
- range,
- result,
- scope,
- );
- result.prefix = prefix;
- result.suffix = suffix;
-
- return result;
+ return {
+ type: 'TextQuoteSelector',
+ exact: range.toString(),
+ ...calculateContextForDisambiguation(range, scope),
+ };
}
function calculateContextForDisambiguation(
range: Range,
- selector: TextQuoteSelector,
- scope: DomScope,
+ scope: Range,
): { prefix?: string; suffix?: string } {
- const exactText = selector.exact;
- const scopeText = rangeFromScope(scope).toString();
+ const exactText = range.toString();
+ const scopeText = scope.toString();
const targetStartIndex = getRangeTextPosition(range, scope);
const targetEndIndex = targetStartIndex + exactText.length;
@@ -153,23 +141,20 @@
}
// Get the index of the first character of range within the text of scope.
-function getRangeTextPosition(range: Range, scope: DomScope): number {
- const scopeAsRange = rangeFromScope(scope);
+function getRangeTextPosition(range: Range, scope: Range): number {
const iter = document.createNodeIterator(
- scopeAsRange.commonAncestorContainer,
+ scope.commonAncestorContainer,
NodeFilter.SHOW_TEXT,
{
acceptNode(node: Text) {
// Only reveal nodes within the range
- return scopeAsRange.intersectsNode(node)
+ return scope.intersectsNode(node)
? NodeFilter.FILTER_ACCEPT
: NodeFilter.FILTER_REJECT;
},
},
);
- const scopeOffset = isTextNode(scopeAsRange.startContainer)
- ? scopeAsRange.startOffset
- : 0;
+ const scopeOffset = isTextNode(scope.startContainer) ? scope.startOffset : 0;
if (isTextNode(range.startContainer))
return seek(iter, range.startContainer) + range.startOffset - scopeOffset;
else return seek(iter, firstTextNodeInRange(range)) - scopeOffset;
diff --git a/packages/dom/src/text-quote/index.ts b/packages/dom/src/text-quote/index.ts
index bb73732..8cd40b3 100644
--- a/packages/dom/src/text-quote/index.ts
+++ b/packages/dom/src/text-quote/index.ts
@@ -20,3 +20,5 @@
export * from './describe';
export * from './match';
+
+export type { TextQuoteSelector } from './selector';
diff --git a/packages/dom/src/text-quote/match.ts b/packages/dom/src/text-quote/match.ts
index a78d057..2cf1567 100644
--- a/packages/dom/src/text-quote/match.ts
+++ b/packages/dom/src/text-quote/match.ts
@@ -18,19 +18,20 @@
* under the License.
*/
-import type { TextQuoteSelector } from '@annotator/selector';
import seek from 'dom-seek';
-import type { DomScope, DomMatcher } from '../types';
-import { ownerDocument, rangeFromScope } from '../scope';
+import type { TextQuoteSelector } from './selector';
export function createTextQuoteSelectorMatcher(
selector: TextQuoteSelector,
-): DomMatcher {
- return async function* matchAll(scope: DomScope) {
- const document = ownerDocument(scope);
- const scopeAsRange = rangeFromScope(scope);
- const scopeText = scopeAsRange.toString();
+): (scope: Range) => AsyncIterable<Range> {
+ return async function* matchAll(scope) {
+ const { commonAncestorContainer, startContainer, startOffset } = scope;
+ const document =
+ commonAncestorContainer.ownerDocument ??
+ (commonAncestorContainer as Document);
+
+ const scopeText = scope.toString();
const exact = selector.exact;
const prefix = selector.prefix || '';
@@ -38,12 +39,12 @@
const searchPattern = prefix + exact + suffix;
const iter = document.createNodeIterator(
- scopeAsRange.commonAncestorContainer,
+ commonAncestorContainer,
NodeFilter.SHOW_TEXT,
{
acceptNode(node: Text) {
// Only reveal nodes within the range; and skip any empty text nodes.
- return scopeAsRange.intersectsNode(node) && node.length > 0
+ return scope.intersectsNode(node) && node.length > 0
? NodeFilter.FILTER_ACCEPT
: NodeFilter.FILTER_REJECT;
},
@@ -51,9 +52,7 @@
);
// The index of the first character of iter.referenceNode inside the text.
- let referenceNodeIndex = isTextNode(scopeAsRange.startContainer)
- ? -scopeAsRange.startOffset
- : 0;
+ let referenceNodeIndex = isTextNode(startContainer) ? -startOffset : 0;
let fromIndex = 0;
while (fromIndex <= scopeText.length) {
diff --git a/packages/dom/src/types.ts b/packages/dom/src/text-quote/selector.ts
similarity index 84%
rename from packages/dom/src/types.ts
rename to packages/dom/src/text-quote/selector.ts
index 9bfc80a..0fcdd42 100644
--- a/packages/dom/src/types.ts
+++ b/packages/dom/src/text-quote/selector.ts
@@ -18,8 +18,9 @@
* under the License.
*/
-import type { Matcher } from '@annotator/selector';
-
-export type DomScope = Node | Range;
-
-export type DomMatcher = Matcher<DomScope, Range>;
+export interface TextQuoteSelector {
+ type: 'TextQuoteSelector';
+ exact: string;
+ prefix?: string;
+ suffix?: string;
+}
diff --git a/packages/dom/test/text-quote/describe-cases.ts b/packages/dom/test/text-quote/describe-cases.ts
index 7d43a5f..987aa1d 100644
--- a/packages/dom/test/text-quote/describe-cases.ts
+++ b/packages/dom/test/text-quote/describe-cases.ts
@@ -18,8 +18,7 @@
* under the License.
*/
-import type { TextQuoteSelector } from '@annotator/selector';
-
+import type { TextQuoteSelector } from '../../src';
import { RangeInfo } from '../utils';
export const testCases: {
diff --git a/packages/dom/test/text-quote/describe.test.ts b/packages/dom/test/text-quote/describe.test.ts
index 9219147..3d36c4e 100644
--- a/packages/dom/test/text-quote/describe.test.ts
+++ b/packages/dom/test/text-quote/describe.test.ts
@@ -32,7 +32,9 @@
for (const [name, { html, range, expected }] of Object.entries(testCases)) {
it(`works for case: ${name}`, async () => {
const doc = domParser.parseFromString(html, 'text/html');
- const result = await describeTextQuote(hydrateRange(range, doc), doc);
+ const scope = doc.createRange();
+ scope.selectNodeContents(doc);
+ const result = await describeTextQuote(hydrateRange(range, doc), scope);
assert.deepEqual(result, expected);
});
}
@@ -85,9 +87,11 @@
for (const [name, { html, selector, expected }] of applicableTestCases) {
it(`case: '${name}'`, async () => {
const doc = domParser.parseFromString(html, 'text/html');
+ const scope = doc.createRange();
+ scope.selectNodeContents(doc);
for (const rangeInfo of expected) {
const range = hydrateRange(rangeInfo, doc);
- const result = await describeTextQuote(range, doc);
+ const result = await describeTextQuote(range, scope);
assert.equal(result.exact, selector.exact);
// Our result may have a different combination of prefix/suffix; only check for obvious inconsistency.
if (selector.prefix && result.prefix)
diff --git a/packages/dom/test/text-quote/match-cases.ts b/packages/dom/test/text-quote/match-cases.ts
index 099802c..142cfc2 100644
--- a/packages/dom/test/text-quote/match-cases.ts
+++ b/packages/dom/test/text-quote/match-cases.ts
@@ -18,8 +18,7 @@
* under the License.
*/
-import type { TextQuoteSelector } from '@annotator/selector';
-
+import type { TextQuoteSelector } from '../../src';
import { RangeInfo } from '../utils';
export const testCases: {
diff --git a/packages/dom/test/text-quote/match.test.ts b/packages/dom/test/text-quote/match.test.ts
index 78b1239..ee8b1ef 100644
--- a/packages/dom/test/text-quote/match.test.ts
+++ b/packages/dom/test/text-quote/match.test.ts
@@ -19,10 +19,9 @@
*/
import { assert } from 'chai';
-import type { TextQuoteSelector } from '@annotator/selector';
+import type { TextQuoteSelector } from '../../src';
import { createTextQuoteSelectorMatcher } from '../../src/text-quote/match';
-import type { DomScope } from '../../src/types';
import { evaluateXPath, RangeInfo } from '../utils';
import { testCases } from './match-cases';
@@ -35,13 +34,21 @@
)) {
it(`works for case: '${name}'`, async () => {
const doc = domParser.parseFromString(html, 'text/html');
- await testMatcher(doc, doc, selector, expected);
+
+ const scope = doc.createRange();
+ scope.selectNodeContents(doc);
+
+ await testMatcher(doc, scope, selector, expected);
});
}
it('handles adjacent text nodes', async () => {
const { html, selector } = testCases['simple'];
const doc = domParser.parseFromString(html, 'text/html');
+
+ const scope = doc.createRange();
+ scope.selectNodeContents(doc);
+
const textNode = evaluateXPath(doc, '//b/text()') as Text;
for (let index = textNode.length - 1; index > 0; index--)
@@ -49,7 +56,7 @@
// console.log([...textNode.parentNode.childNodes].map(node => node.textContent))
// → 'l', 'o', 'r', 'e', 'm', …
- await testMatcher(doc, doc, selector, [
+ await testMatcher(doc, scope, selector, [
{
startContainerXPath: '//b/text()[13]',
startOffset: 0,
@@ -63,6 +70,9 @@
const { html, selector } = testCases['simple'];
const doc = domParser.parseFromString(html, 'text/html');
+ const scope = doc.createRange();
+ scope.selectNodeContents(doc);
+
const textNode = evaluateXPath(doc, '//b/text()') as Text;
textNode.splitText(textNode.length);
textNode.splitText(20);
@@ -75,7 +85,7 @@
// console.log([...textNode.parentNode.childNodes].map(node => node.textContent))
// → '', 'lorem ipsum ', '', 'dolor', '', ' am', '', 'et yada yada', ''
- await testMatcher(doc, doc, selector, [
+ await testMatcher(doc, scope, selector, [
{
startContainerXPath: '//b/text()[4]', // "dolor"
startOffset: 0,
@@ -89,24 +99,33 @@
const { html, selector, expected } = testCases['simple'];
const doc = domParser.parseFromString(html, 'text/html');
- await testMatcher(doc, evaluateXPath(doc, '//b'), selector, expected);
+ const scope = doc.createRange();
+ scope.selectNodeContents(evaluateXPath(doc, '//b'));
+
+ await testMatcher(doc, scope, selector, expected);
});
it('works with parent of text as scope, when matching its first characters', async () => {
const { html, selector, expected } = testCases['first characters'];
const doc = domParser.parseFromString(html, 'text/html');
- await testMatcher(doc, evaluateXPath(doc, '//b'), selector, expected);
+ const scope = doc.createRange();
+ scope.selectNodeContents(evaluateXPath(doc, '//b'));
+
+ await testMatcher(doc, scope, selector, expected);
});
it('works with parent of text as scope, when matching its first characters, with an empty text node', async () => {
const { html, selector } = testCases['first characters'];
const doc = domParser.parseFromString(html, 'text/html');
+ const scope = doc.createRange();
+ scope.selectNodeContents(evaluateXPath(doc, '//b'));
+
const textNode = evaluateXPath(doc, '//b/text()') as Text;
textNode.splitText(0);
- await testMatcher(doc, evaluateXPath(doc, '//b'), selector, [
+ await testMatcher(doc, scope, selector, [
{
startContainerXPath: '//b/text()[2]',
startOffset: 0,
@@ -179,7 +198,7 @@
async function testMatcher(
doc: Document,
- scope: DomScope,
+ scope: Range,
selector: TextQuoteSelector,
expected: RangeInfo[],
) {
diff --git a/packages/selector/src/index.ts b/packages/selector/src/index.ts
index c66bd94..99c2840 100644
--- a/packages/selector/src/index.ts
+++ b/packages/selector/src/index.ts
@@ -18,39 +18,46 @@
* under the License.
*/
-import type { Matcher, Selector } from './types';
-
-export type { Matcher, Selector } from './types';
-export type { CssSelector, RangeSelector, TextQuoteSelector } from './types';
-
-export function makeRefinable<
- // Any subtype of Selector can be made refinable; but note we limit the value
- // of refinedBy because it must also be accepted by matcherCreator.
- TSelector extends Selector & { refinedBy: TSelector },
- TScope,
- // To enable refinement, the implementation’s Match object must be usable as a
- // Scope object itself.
- TMatch extends TScope
+export function withRefinement<
+ TSelector,
+ TSelectorScope,
+ TRefinement,
+ TRefinementScope,
+ TMatch
>(
- matcherCreator: (selector: TSelector) => Matcher<TScope, TMatch>,
-): (selector: TSelector) => Matcher<TScope, TMatch> {
- return function createMatcherWithRefinement(
- sourceSelector: TSelector,
- ): Matcher<TScope, TMatch> {
- const matcher = matcherCreator(sourceSelector);
+ createMatcher: (
+ selector: TSelector,
+ ) => (scope: TSelectorScope) => AsyncIterable<TRefinementScope>,
+ createRefiner: (
+ selector: TRefinement,
+ ) => (scope: TRefinementScope) => AsyncIterable<TMatch>,
+): (
+ selector: TSelector & { refinedBy?: TRefinement },
+) => (scope: TSelectorScope) => AsyncIterable<TRefinementScope | TMatch> {
+ return function createMatcherWithRefinement(selector) {
+ const { refinedBy } = selector;
- if (sourceSelector.refinedBy) {
- const refiningSelector = createMatcherWithRefinement(
- sourceSelector.refinedBy,
- );
+ if (refinedBy) {
+ const match = createMatcher(selector);
+ const refine = createRefiner(refinedBy);
return async function* matchAll(scope) {
- for await (const match of matcher(scope)) {
- yield* refiningSelector(match);
+ for await (const subScope of match(scope)) {
+ yield* refine(subScope);
}
};
}
- return matcher;
+ return createMatcher(selector);
};
}
+
+export function withRecursiveRefinement<TSelector, TScope>(
+ createMatcher: (
+ selector: TSelector & { refinedBy?: TSelector },
+ ) => (scope: TScope) => AsyncIterable<TScope>,
+): (
+ selector: TSelector & { refinedBy?: TSelector },
+) => (scope: TScope) => AsyncIterable<TScope> {
+ return withRefinement(createMatcher, createMatcher);
+}
diff --git a/packages/selector/src/types.ts b/packages/selector/src/types.ts
deleted file mode 100644
index fc4f64b..0000000
--- a/packages/selector/src/types.ts
+++ /dev/null
@@ -1,45 +0,0 @@
-/**
- * @license
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-export interface Selector {
- refinedBy?: Selector;
-}
-
-export interface CssSelector extends Selector {
- type: 'CssSelector';
- value: string;
-}
-
-export interface TextQuoteSelector extends Selector {
- type: 'TextQuoteSelector';
- exact: string;
- prefix?: string;
- suffix?: string;
-}
-
-export interface RangeSelector extends Selector {
- type: 'RangeSelector';
- startSelector: Selector;
- endSelector: Selector;
-}
-
-export interface Matcher<TScope, TMatch> {
- (scope: TScope): AsyncGenerator<TMatch, void, void>;
-}
diff --git a/web/demo/index.js b/web/demo/index.js
index 6de6493..18c9983 100644
--- a/web/demo/index.js
+++ b/web/demo/index.js
@@ -26,7 +26,7 @@
describeTextQuote,
highlightRange,
} from '@annotator/dom';
-import { makeRefinable } from '@annotator/selector';
+import { withRecursiveRefinement } from '@annotator/selector';
const EXAMPLE_SELECTORS = [
{
@@ -91,7 +91,7 @@
target.normalize();
}
-const createMatcher = makeRefinable((selector) => {
+const createMatcher = withRecursiveRefinement((selector) => {
const innerCreateMatcher = {
TextQuoteSelector: createTextQuoteSelectorMatcher,
RangeSelector: makeCreateRangeSelectorMatcher(createMatcher),
@@ -105,10 +105,13 @@
});
async function anchor(selector) {
+ const scope = document.createRange();
+ scope.selectNodeContents(target);
+
const matchAll = createMatcher(selector);
const ranges = [];
- for await (const range of matchAll(target)) {
+ for await (const range of matchAll(scope)) {
ranges.push(range);
}
