api(selectors): pass selector name when registering, allow file path (#1162)

Author: dgozman

docs/api.md

@@ -3149,12 +3149,14 @@ Contains the URL of the response.
 Selectors can be used to install custom selector engines. See [Working with selectors](#working-with-selectors) for more information.
 
 <!-- GEN:toc -->
-- [selectors.register(engineFunction[, ...args])](#selectorsregisterenginefunction-args)
+- [selectors.register(name, script)](#selectorsregistername-script)
 <!-- GEN:stop -->
 
-#### selectors.register(engineFunction[, ...args])
-- `engineFunction` <[function]|[string]> Function that evaluates to a selector engine instance.
-- `...args` <...[Serializable]> Arguments to pass to `engineFunction`.
+#### selectors.register(name, script)
+- `name` <[string]> Name that is used in selectors as a prefix, e.g. `{name: 'foo'}` enables `foo=myselectorbody` selectors. May only contain `[a-zA-Z0-9_]` characters.
+- `script` <[function]|[string]|[Object]> Script that evaluates to a selector engine instance.
+  - `path` <[string]> Path to the JavaScript file. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd).
+  - `content` <[string]> Raw script content.
 - returns: <[Promise]>
 
 An example of registering selector engine that queries elements based on a tag name:
@@ -3164,9 +3166,6 @@ const { selectors, firefox } = require('playwright');  // Or 'chromium' or 'webk
 (async () => {
   // Must be a function that evaluates to a selector engine instance.
   const createTagNameEngine = () => ({
-    // Selectors will be prefixed with "tag=".
-    name: 'tag',
-
     // Creates a selector that matches given target when queried at the root.
     // Can return undefined if unable to create one.
     create(root, target) {
@@ -3184,8 +3183,8 @@ const { selectors, firefox } = require('playwright');  // Or 'chromium' or 'webk
     }
   });
 
-  // Register the engine.
-  await selectors.register(createTagNameEngine);
+  // Register the engine. Selectors will be prefixed with "tag=".
+  await selectors.register('tag', createTagNameEngine);
 
   const browser = await firefox.launch();
   const page = await browser.newPage();

docs/selectors.md

@@ -84,11 +84,10 @@ Id engines are selecting based on the corresponding atrribute value. For example
 
 ## Custom selector engines
 
-Playwright supports custom selector engines, registered with [selectors.register(engineFunction[, ...args])](api.md#selectorsregisterenginefunction-args).
+Playwright supports custom selector engines, registered with [selectors.register(name, script)](api.md#selectorsregistername-script).
 
 Selector engine should have the following properties:
 
-- `name` Selector name used in selector strings.
 - `create` Function to create a relative selector from `root` (root is either a `Document`, `ShadowRoot` or `Element`) to a `target` element.
 - `query` Function to query first element matching `selector` relative to the `root`.
 - `queryAll` Function to query all elements matching `selector` relative to the `root`.
@@ -97,9 +96,6 @@ An example of registering selector engine that queries elements based on a tag n
 ```js
 // Must be a function that evaluates to a selector engine instance.
 const createTagNameEngine = () => ({
-  // Selectors will be prefixed with "tag=".
-  name: 'tag',
-
   // Creates a selector that matches given target when queried at the root.
   // Can return undefined if unable to create one.
   create(root, target) {
@@ -117,8 +113,8 @@ const createTagNameEngine = () => ({
   }
 });
 
-// Register the engine.
-await selectors.register(createTagNameEngine);
+// Register the engine. Selectors will be prefixed with "tag=".
+await selectors.register('tag', createTagNameEngine);
 
 // Now we can use 'tag=' selectors.
 const button = await page.$('tag=button');

src/chromium/crBrowser.ts

@@ -303,7 +303,7 @@ export class CRBrowserContext extends platform.EventEmitter implements BrowserCo
   }
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) {
-    const source = await helper.evaluationScript(script, ...args);
+    const source = await helper.evaluationScript(script, args);
     this._evaluateOnNewDocumentSources.push(source);
     for (const page of this._existingPages())
       await (page._delegate as CRPage).evaluateOnNewDocument(source);

src/dom.ts

@@ -85,9 +85,12 @@ export class FrameExecutionContext extends js.ExecutionContext {
       this._injectedPromise = undefined;
     }
     if (!this._injectedPromise) {
+      const custom: string[] = [];
+      for (const [name, source] of selectors._engines)
+        custom.push(`{ name: '${name}', engine: (${source}) }`);
       const source = `
         new (${injectedSource.source})([
-          ${selectors._sources.join(',\n')}
+          ${custom.join(',\n')}
         ])
       `;
       this._injectedPromise = this.evaluateHandle(source);

src/firefox/ffBrowser.ts

@@ -360,7 +360,7 @@ export class FFBrowserContext extends platform.EventEmitter implements BrowserCo
   }
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) {
-    const source = await helper.evaluationScript(script, ...args);
+    const source = await helper.evaluationScript(script, args);
     this._evaluateOnNewDocumentSources.push(source);
     await this._browser._connection.send('Browser.addScriptToEvaluateOnNewDocument', { browserContextId: this._browserContextId || undefined, script: source });
   }

src/helper.ts

@@ -41,13 +41,14 @@ class Helper {
     }
   }
 
-  static async evaluationScript(fun: Function | string | { path?: string, content?: string }, ...args: any[]): Promise<string> {
+  static async evaluationScript(fun: Function | string | { path?: string, content?: string }, args: any[] = [], addSourceUrl: boolean = true): Promise<string> {
     if (!helper.isString(fun) && typeof fun !== 'function') {
       if (fun.content !== undefined) {
         fun = fun.content;
       } else if (fun.path !== undefined) {
         let contents = await platform.readFileAsync(fun.path, 'utf8');
-        contents += '//# sourceURL=' + fun.path.replace(/\n/g, '');
+        if (addSourceUrl)
+          contents += '//# sourceURL=' + fun.path.replace(/\n/g, '');
         fun = contents;
       } else {
         throw new Error('Either path or content property must be present');

src/injected/cssSelectorEngine.ts

@@ -17,8 +17,6 @@
 import { SelectorEngine, SelectorRoot } from './selectorEngine';
 
 export const CSSEngine: SelectorEngine = {
-  name: 'css',
-
   create(root: SelectorRoot, targetElement: Element): string | undefined {
     const tokens: string[] = [];
 

src/injected/injected.ts

@@ -23,8 +23,6 @@ import * as types from '../types';
 
 function createAttributeEngine(attribute: string): SelectorEngine {
   const engine: SelectorEngine = {
-    name: attribute,
-
     create(root: SelectorRoot, target: Element): string | undefined {
       const value = target.getAttribute(attribute);
       if (!value)
@@ -51,20 +49,19 @@ class Injected {
   readonly utils: Utils;
   readonly engines: Map<string, SelectorEngine>;
 
-  constructor(customEngines: SelectorEngine[]) {
-    const defaultEngines = [
-      CSSEngine,
-      XPathEngine,
-      TextEngine,
-      createAttributeEngine('id'),
-      createAttributeEngine('data-testid'),
-      createAttributeEngine('data-test-id'),
-      createAttributeEngine('data-test'),
-    ];
+  constructor(customEngines: { name: string, engine: SelectorEngine}[]) {
     this.utils = new Utils();
     this.engines = new Map();
-    for (const engine of [...defaultEngines, ...customEngines])
-      this.engines.set(engine.name, engine);
+    // Note: keep predefined names in sync with Selectors class.
+    this.engines.set('css', CSSEngine);
+    this.engines.set('xpath', XPathEngine);
+    this.engines.set('text', TextEngine);
+    this.engines.set('id', createAttributeEngine('id'));
+    this.engines.set('data-testid', createAttributeEngine('data-testid'));
+    this.engines.set('data-test-id', createAttributeEngine('data-test-id'));
+    this.engines.set('data-test', createAttributeEngine('data-test'));
+    for (const {name, engine} of customEngines)
+      this.engines.set(name, engine);
   }
 
   querySelector(selector: string, root: Node): Element | undefined {

src/injected/selectorEngine.ts

@@ -18,7 +18,6 @@ export type SelectorType = 'default' | 'notext';
 export type SelectorRoot = Element | ShadowRoot | Document;
 
 export interface SelectorEngine {
-  name: string;
   create(root: SelectorRoot, target: Element, type?: SelectorType): string | undefined;
   query(root: SelectorRoot, selector: string): Element | undefined;
   queryAll(root: SelectorRoot, selector: string): Element[];

src/injected/textSelectorEngine.ts

@@ -17,8 +17,6 @@
 import { SelectorEngine, SelectorType, SelectorRoot } from './selectorEngine';
 
 export const TextEngine: SelectorEngine = {
-  name: 'text',
-
   create(root: SelectorRoot, targetElement: Element, type: SelectorType): string | undefined {
     const document = root instanceof Document ? root : root.ownerDocument;
     if (!document)

src/injected/xpathSelectorEngine.ts

@@ -20,8 +20,6 @@ const maxTextLength = 80;
 const minMeaningfulSelectorLegth = 100;
 
 export const XPathEngine: SelectorEngine = {
-  name: 'xpath',
-
   create(root: SelectorRoot, targetElement: Element, type: SelectorType): string | undefined {
     const maybeDocument = root instanceof Document ? root : root.ownerDocument;
     if (!maybeDocument)

src/injected/zsSelectorEngine.ts

@@ -751,8 +751,6 @@ class Engine {
 }
 
 const ZSSelectorEngine: SelectorEngine = {
-  name: 'zs',
-
   create(root: SelectorRoot, element: Element, type?: SelectorType): string {
     return new Engine().create(root, element, type || 'default');
   },

src/page.ts

@@ -412,7 +412,7 @@ export class Page extends platform.EventEmitter {
   }
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) {
-    await this._delegate.evaluateOnNewDocument(await helper.evaluationScript(script, ...args));
+    await this._delegate.evaluateOnNewDocument(await helper.evaluationScript(script, args));
   }
 
   async setCacheEnabled(enabled: boolean = true) {

src/selectors.ts

@@ -21,7 +21,7 @@ import { helper } from './helper';
 let selectors: Selectors;
 
 export class Selectors {
-  readonly _sources: string[];
+  readonly _engines: Map<string, string>;
   _generation = 0;
 
   static _instance() {
@@ -31,12 +31,19 @@ export class Selectors {
   }
 
   constructor() {
-    this._sources = [];
+    this._engines = new Map();
   }
 
-  async register(engineFunction: string | Function, ...args: any[]) {
-    const source = helper.evaluationString(engineFunction, ...args);
-    this._sources.push(source);
+  async register(name: string, script: string | Function | { path?: string, content?: string }): Promise<void> {
+    if (!name.match(/^[a-zA-Z_0-9-]+$/))
+      throw new Error('Selector engine name may only contain [a-zA-Z0-9_] characters');
+    // Note: keep in sync with Injected class, and also keep 'zs' for future.
+    if (['css', 'xpath', 'text', 'id', 'zs', 'data-testid', 'data-test-id', 'data-test'].includes(name))
+      throw new Error(`"${name}" is a predefined selector engine`);
+    const source = await helper.evaluationScript(script, [], false);
+    if (this._engines.has(name))
+      throw new Error(`"${name}" selector engine has been already registered`);
+    this._engines.set(name, source);
     ++this._generation;
   }
 

src/webkit/wkBrowser.ts

@@ -279,7 +279,7 @@ export class WKBrowserContext extends platform.EventEmitter implements BrowserCo
   }
 
   async addInitScript(script: Function | string | { path?: string, content?: string }, ...args: any[]) {
-    const source = await helper.evaluationScript(script, ...args);
+    const source = await helper.evaluationScript(script, args);
     this._evaluateOnNewDocumentSources.push(source);
     for (const page of this._existingPages())
       await (page._delegate as WKPage)._updateBootstrapScript();

test/assets/sectionselectorengine.js

@@ -0,0 +1,10 @@
+({
+  create(root, target) {
+  },
+  query(root, selector) {
+    return root.querySelector('section');
+  },
+  queryAll(root, selector) {
+    return Array.from(root.querySelectorAll('section'));
+  }
+})