From 8f480a398743eeee1f9f3b0c4314c731ba02101d Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Fri, 20 Dec 2019 16:57:21 -0800 Subject: [PATCH] docs: more docs update (#327) --- docs/api.md | 102 +++++++++++++++--- src/frames.ts | 10 +- src/page.ts | 4 +- src/types.ts | 5 +- test/waittask.spec.js | 29 ++--- .../doclint/check_public_api/Documentation.js | 44 ++++++++ utils/doclint/check_public_api/MDBuilder.js | 3 + utils/doclint/check_public_api/index.js | 54 ---------- 8 files changed, 150 insertions(+), 101 deletions(-) diff --git a/docs/api.md b/docs/api.md index 76156ea44c..46c5bd3e41 100644 --- a/docs/api.md +++ b/docs/api.md @@ -91,6 +91,7 @@ * [page.content()](#pagecontent) * [page.coverage](#pagecoverage) * [page.dblclick(selector[, options])](#pagedblclickselector-options) + * [page.emulateMedia(options)](#pageemulatemediaoptions) * [page.evaluate(pageFunction[, ...args])](#pageevaluatepagefunction-args) * [page.evaluateHandle(pageFunction[, ...args])](#pageevaluatehandlepagefunction-args) * [page.evaluateOnNewDocument(pageFunction[, ...args])](#pageevaluateonnewdocumentpagefunction-args) @@ -116,10 +117,12 @@ * [page.setDefaultNavigationTimeout(timeout)](#pagesetdefaultnavigationtimeouttimeout) * [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) * [page.setExtraHTTPHeaders(headers)](#pagesetextrahttpheadersheaders) + * [page.setViewport(viewport)](#pagesetviewportviewport) * [page.title()](#pagetitle) * [page.tripleclick(selector[, options])](#pagetripleclickselector-options) * [page.type(selector, text[, options])](#pagetypeselector-text-options) * [page.url()](#pageurl) + * [page.viewport()](#pageviewport) * [page.waitFor(selectorOrFunctionOrTimeout[, options[, ...args]])](#pagewaitforselectororfunctionortimeout-options-args) * [page.waitForFunction(pageFunction[, options[, ...args]])](#pagewaitforfunctionpagefunction-options-args) * [page.waitForNavigation([options])](#pagewaitfornavigationoptions) @@ -257,7 +260,6 @@ * [response.text()](#responsetext) * [response.url()](#responseurl) - [class: ChromiumTarget](#class-chromiumtarget) - * [chromiumTarget.browser()](#chromiumtargetbrowser) * [chromiumTarget.browserContext()](#chromiumtargetbrowsercontext) * [chromiumTarget.createCDPSession()](#chromiumtargetcreatecdpsession) * [chromiumTarget.opener()](#chromiumtargetopener) @@ -792,7 +794,7 @@ await browserContext.setCookies([cookieObject1, cookieObject2]); #### browserContext.setPermissions(origin, permissions[]) - `origin` <[string]> The [origin] to grant permissions to, e.g. "https://example.com". -- `permissions` <[Array]<[string]>> An array of permissions to grant. All permissions that are not listed here will be automatically denied. Permissions can be one of the following values: +- `permissions` <[Array]> An array of permissions to grant. All permissions that are not listed here will be automatically denied. Permissions can be one of the following values: - `'geolocation'` - `'midi'` - `'midi-sysex'` (system-exclusive midi) @@ -1142,6 +1144,41 @@ Bear in mind that if the first click of the `dblclick()` triggers a navigation e Shortcut for [page.mainFrame().dblclick(selector[, options])](#framedblclickselector-options). +#### page.emulateMedia(options) +- `options` <[Object]> + - `type` <"screen"|"print"> Changes the CSS media type of the page. The only allowed values are `'screen'`, `'print'` and `null`. Passing `null` disables CSS media emulation. + - `colorScheme` <"dark"|"light"|"no-preference"> Emulates `'prefers-colors-scheme'` media feature, supported values are `'light'`, `'dark'`, `'no-preference'`. +- returns: <[Promise]> + +```js +await page.evaluate(() => matchMedia('screen').matches)); +// → true +await page.evaluate(() => matchMedia('print').matches)); +// → true + +await page.emulateMedia({ type: 'print' }); +await page.evaluate(() => matchMedia('screen').matches)); +// → false +await page.evaluate(() => matchMedia('print').matches)); +// → true + +await page.emulateMedia({}); +await page.evaluate(() => matchMedia('screen').matches)); +// → true +await page.evaluate(() => matchMedia('print').matches)); +// → true +``` + +```js +await page.emulateMedia({ colorScheme: 'dark' }] }); +await page.evaluate(() => matchMedia('(prefers-color-scheme: dark)').matches)); +// → true +await page.evaluate(() => matchMedia('(prefers-color-scheme: light)').matches)); +// → false +await page.evaluate(() => matchMedia('(prefers-color-scheme: no-preference)').matches)); +// → false +``` + #### page.evaluate(pageFunction[, ...args]) - `pageFunction` <[function]|[string]> Function to be evaluated in the page context - `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction` @@ -1525,6 +1562,32 @@ The extra HTTP headers will be sent with every request the page initiates. > **NOTE** page.setExtraHTTPHeaders does not guarantee the order of headers in the outgoing requests. +#### page.setViewport(viewport) +- `viewport` <[Object]> + - `width` <[number]> page width in pixels. **required** + - `height` <[number]> page height in pixels. **required** + - `deviceScaleFactor` <[number]> Specify device scale factor (can be thought of as dpr). Defaults to `1`. + - `isMobile` <[boolean]> Whether the `meta viewport` tag is taken into account. Defaults to `false`. + - `hasTouch`<[boolean]> Specifies if viewport supports touch events. Defaults to `false` + - `isLandscape` <[boolean]> Specifies if viewport is in landscape mode. Defaults to `false`. +- returns: <[Promise]> + +> **NOTE** in certain cases, setting viewport will reload the page in order to set the `isMobile` or `hasTouch` properties. + +In the case of multiple pages in a single browser, each page can have its own viewport size. + +`page.setViewport` will resize the page. A lot of websites don't expect phones to change size, so you should set the viewport before navigating to the page. + +```js +const page = await browser.newPage(); +await page.setViewport({ + width: 640, + height: 480, + deviceScaleFactor: 1, +}); +await page.goto('https://example.com'); +``` + #### page.title() - returns: <[Promise]<[string]>> The page's title. @@ -1577,15 +1640,23 @@ Shortcut for [page.mainFrame().type(selector, text[, options])](#frametypeselect This is a shortcut for [page.mainFrame().url()](#frameurl) +#### page.viewport() +- returns: + - `width` <[number]> page width in pixels. + - `height` <[number]> page height in pixels. + - `deviceScaleFactor` <[number]> Specify device scale factor (can be though of as dpr). Defaults to `1`. + - `isMobile` <[boolean]> Whether the `meta viewport` tag is taken into account. Defaults to `false`. + - `hasTouch`<[boolean]> Specifies if viewport supports touch events. Defaults to `false` + - `isLandscape` <[boolean]> Specifies if viewport is in landscape mode. Defaults to `false`. + #### page.waitFor(selectorOrFunctionOrTimeout[, options[, ...args]]) - `selectorOrFunctionOrTimeout` <[string]|[number]|[function]> A [selector], predicate or timeout to wait for - `options` <[Object]> Optional waiting parameters - - `visible` <[boolean]> wait for element to be present in DOM and to be visible. Defaults to `false`. - - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - - `hidden` <[boolean]> wait for element to not be found in the DOM or to be hidden. Defaults to `false`. - - `polling` <[string]|[number]> An interval at which the `pageFunction` is executed, defaults to `raf`. If `polling` is a number, then it is treated as an interval in milliseconds at which the function would be executed. If `polling` is a string, then it can be one of the following values: + - `visibility` <"visible"|"hidden"|"any"> Wait for element to become visible (`visible`), hidden (`hidden`), present in dom (`any`). Defaults to `any`. + - `polling` <[number]|"raf"|"mutation"> An interval at which the `pageFunction` is executed, defaults to `raf`. If `polling` is a number, then it is treated as an interval in milliseconds at which the function would be executed. If `polling` is a string, then it can be one of the following values: - `raf` - to constantly execute `pageFunction` in `requestAnimationFrame` callback. This is the tightest polling mode which is suitable to observe styling changes. - `mutation` - to execute `pageFunction` on every DOM mutation. + - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction` - returns: <[Promise]<[JSHandle]>> Promise which resolves to a JSHandle of the success value @@ -1704,7 +1775,7 @@ return finalResponse.ok(); #### page.waitForSelector(selector[, options]) - `selector` <[string]> A selector of an element to wait for - `options` <[Object]> - - `waitFor` <"visible"|"hidden"|"any"|"nowait"> Wait for element to become visible (`visible`), hidden (`hidden`), present in dom (`any`) or do not wait at all (`nowait`). Defaults to `visible`. + - `visibility` <"visible"|"hidden"|"any"> Wait for element to become visible (`visible`), hidden (`hidden`), present in dom (`any`). Defaults to `any`. - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - returns: <[Promise]> Promise which resolves when element specified by selector string is added to DOM. Resolves to `null` if waiting for `hidden: true` and selector is not found in DOM. @@ -1999,7 +2070,6 @@ Shortcut for [`mouse.move`](#mousemovex-y-options), [`mouse.down`](#mousedownopt - `relativePoint` <[Object]> Optional relative point - `x` <[number]> x coordinate - `y` <[number]> y coordinate - - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - returns: <[Promise]> Shortcut for [`mouse.move`](#mousemovex-y-options), [`mouse.down`](#mousedownoptions), [`mouse.up`](#mouseupoptions), [`mouse.down`](#mousedownoptions) and [`mouse.up`](#mouseupoptions). @@ -2031,7 +2101,6 @@ Dispatches a `mousemove` event. - `relativePoint` <[Object]> Optional relative point - `x` <[number]> x coordinate - `y` <[number]> y coordinate - - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - returns: <[Promise]> Shortcut for [`mouse.move`](#mousemovex-y-options), [`mouse.down`](#mousedownoptions), [`mouse.up`](#mouseupoptions), [`mouse.down`](#mousedownoptions), [`mouse.up`](#mouseupoptions), [`mouse.down`](#mousedownoptions) and [`mouse.up`](#mouseupoptions). @@ -2080,7 +2149,7 @@ Dispatches a `mouseup` event. ```js // Generates a PDF with 'screen' media type. -await page.emulateMedia({ type: 'screen' }); +await page.emulateMedia({type: 'screen'}); await page.pdf.generate({path: 'page.pdf'}); ``` @@ -2569,6 +2638,11 @@ Returns frame's url. #### frame.waitFor(selectorOrFunctionOrTimeout[, options[, ...args]]) - `selectorOrFunctionOrTimeout` <[string]|[number]|[function]> A [selector], predicate or timeout to wait for - `options` <[Object]> Optional waiting parameters + - `visibility` <"visible"|"hidden"|"any"> Wait for element to become visible (`visible`), hidden (`hidden`), present in dom (`any`). Defaults to `any`. + - `polling` <[number]|"raf"|"mutation"> An interval at which the `pageFunction` is executed, defaults to `raf`. If `polling` is a number, then it is treated as an interval in milliseconds at which the function would be executed. If `polling` is a string, then it can be one of the following values: + - `raf` - to constantly execute `pageFunction` in `requestAnimationFrame` callback. This is the tightest polling mode which is suitable to observe styling changes. + - `mutation` - to execute `pageFunction` on every DOM mutation. + - `timeout` <[number]> maximum time to wait for in milliseconds. Defaults to `30000` (30 seconds). Pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - `...args` <...[Serializable]|[JSHandle]> Arguments to pass to `pageFunction` - returns: <[Promise]<[JSHandle]>> Promise which resolves to a JSHandle of the success value @@ -2654,7 +2728,7 @@ const [response] = await Promise.all([ #### frame.waitForSelector(selector[, options]) - `selector` <[string]> A selector of an element to wait for - `options` <[Object]> - - `waitFor` <"visible"|"hidden"|"any"|"nowait"> Wait for element to become visible (`visible`), hidden (`hidden`), present in dom (`any`) or do not wait at all (`nowait`). Defaults to `visible`. + - `visibility` <"visible"|"hidden"|"any"> Wait for element to become visible (`visible`), hidden (`hidden`), present in dom (`any`). Defaults to `any`. - `timeout` <[number]> Maximum navigation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be changed by using the [page.setDefaultTimeout(timeout)](#pagesetdefaulttimeouttimeout) method. - returns: <[Promise]> Promise which resolves when element specified by selector string is added to DOM. Resolves to `null` if waiting for `hidden: true` and selector is not found in DOM. @@ -3268,12 +3342,6 @@ Contains the URL of the response. ### class: ChromiumTarget -#### chromiumTarget.browser() - -- returns: <[Browser]> - -Get the browser the target belongs to. - #### chromiumTarget.browserContext() - returns: <[BrowserContext]> diff --git a/src/frames.ts b/src/frames.ts index b685fc149e..caa8979eb2 100644 --- a/src/frames.ts +++ b/src/frames.ts @@ -383,11 +383,9 @@ export class Frame { return handle; } - async waitForSelector(selector: string, options?: types.TimeoutOptions & { waitFor?: types.Visibility }): Promise | null> { - const { timeout = this._page._timeoutSettings.timeout(), waitFor = 'any' } = (options || {}); - if ((waitFor as any) === 'nowait') - throw new Error('waitForSelector does not support "nowait"'); - const handle = await this._waitForSelectorInUtilityContext(selector, waitFor as types.Visibility, timeout); + async waitForSelector(selector: string, options?: types.TimeoutOptions & { visibility?: types.Visibility }): Promise | null> { + const { timeout = this._page._timeoutSettings.timeout(), visibility = 'any' } = (options || {}); + const handle = await this._waitForSelectorInUtilityContext(selector, visibility, timeout); const mainContext = await this._mainContext(); if (handle && handle._context !== mainContext) { const adopted = this._page._delegate.adoptElementHandle(handle, mainContext); @@ -662,7 +660,7 @@ export class Frame { await handle.dispose(); } - async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options: any = {}, ...args: any[]): Promise { + async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options: types.WaitForFunctionOptions & { visibility?: types.Visibility } = {}, ...args: any[]): Promise { if (helper.isString(selectorOrFunctionOrTimeout)) return this.waitForSelector(selectorOrFunctionOrTimeout as string, options) as any; if (helper.isNumber(selectorOrFunctionOrTimeout)) diff --git a/src/page.ts b/src/page.ts index e31b2ffac1..e1e9a984a7 100644 --- a/src/page.ts +++ b/src/page.ts @@ -162,7 +162,7 @@ export class Page extends EventEmitter { return this.mainFrame().$(selector); } - async waitForSelector(selector: string, options?: types.TimeoutOptions & { waitFor?: types.Visibility }): Promise | null> { + async waitForSelector(selector: string, options?: types.TimeoutOptions & { visibility?: types.Visibility }): Promise | null> { return this.mainFrame().waitForSelector(selector, options); } @@ -445,7 +445,7 @@ export class Page extends EventEmitter { return this.mainFrame().type(selector, text, options); } - async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options: { visible?: boolean; hidden?: boolean; timeout?: number; polling?: string | number; } = {}, ...args: any[]): Promise { + async waitFor(selectorOrFunctionOrTimeout: (string | number | Function), options?: types.WaitForFunctionOptions & { visibility?: types.Visibility }, ...args: any[]): Promise { return this.mainFrame().waitFor(selectorOrFunctionOrTimeout, options, ...args); } diff --git a/src/types.ts b/src/types.ts index 36878d0648..53ebcb15b3 100644 --- a/src/types.ts +++ b/src/types.ts @@ -10,12 +10,11 @@ type PageFunction = string | ((...args: Args) => R type PageFunctionOn = string | ((on: On, ...args: Args) => R | Promise); type Handle = T extends Node ? dom.ElementHandle : js.JSHandle; -type ElementForSelector = T extends keyof HTMLElementTagNameMap ? HTMLElementTagNameMap[T] : Element; export type Evaluate = (pageFunction: PageFunction, ...args: Boxed) => Promise; export type EvaluateHandle = (pageFunction: PageFunction, ...args: Boxed) => Promise>; -export type $Eval = (selector: S, pageFunction: PageFunctionOn, Args, R>, ...args: Boxed) => Promise; -export type $$Eval = (selector: S, pageFunction: PageFunctionOn[], Args, R>, ...args: Boxed) => Promise; +export type $Eval = (selector: string, pageFunction: PageFunctionOn, ...args: Boxed) => Promise; +export type $$Eval = (selector: string, pageFunction: PageFunctionOn, ...args: Boxed) => Promise; export type EvaluateOn = (pageFunction: PageFunctionOn, ...args: Boxed) => Promise; export type EvaluateHandleOn = (pageFunction: PageFunctionOn, ...args: Boxed) => Promise>; diff --git a/test/waittask.spec.js b/test/waittask.spec.js index d35c5898d6..4d169b9a59 100644 --- a/test/waittask.spec.js +++ b/test/waittask.spec.js @@ -331,7 +331,7 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO }); it('should wait for visible recursively', async({page, server}) => { let divVisible = false; - const waitForSelector = page.waitForSelector('div#inner', { waitFor: 'visible' }).then(() => divVisible = true); + const waitForSelector = page.waitForSelector('div#inner', { visibility: 'visible' }).then(() => divVisible = true); await page.setContent(`
hi
`); expect(divVisible).toBe(false); await page.evaluate(() => document.querySelector('div').style.removeProperty('display')); @@ -343,7 +343,7 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO it('hidden should wait for visibility: hidden', async({page, server}) => { let divHidden = false; await page.setContent(`
`); - const waitForSelector = page.waitForSelector('div', { waitFor: 'hidden' }).then(() => divHidden = true); + const waitForSelector = page.waitForSelector('div', { visibility: 'hidden' }).then(() => divHidden = true); await page.waitForSelector('div'); // do a round trip expect(divHidden).toBe(false); await page.evaluate(() => document.querySelector('div').style.setProperty('visibility', 'hidden')); @@ -353,7 +353,7 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO it('hidden should wait for display: none', async({page, server}) => { let divHidden = false; await page.setContent(`
`); - const waitForSelector = page.waitForSelector('div', { waitFor: 'hidden' }).then(() => divHidden = true); + const waitForSelector = page.waitForSelector('div', { visibility: 'hidden' }).then(() => divHidden = true); await page.waitForSelector('div'); // do a round trip expect(divHidden).toBe(false); await page.evaluate(() => document.querySelector('div').style.setProperty('display', 'none')); @@ -363,7 +363,7 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO it('hidden should wait for removal', async({page, server}) => { await page.setContent(`
`); let divRemoved = false; - const waitForSelector = page.waitForSelector('div', { waitFor: 'hidden' }).then(() => divRemoved = true); + const waitForSelector = page.waitForSelector('div', { visibility: 'hidden' }).then(() => divRemoved = true); await page.waitForSelector('div'); // do a round trip expect(divRemoved).toBe(false); await page.evaluate(() => document.querySelector('div').remove()); @@ -371,7 +371,7 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO expect(divRemoved).toBe(true); }); it('should return null if waiting to hide non-existing element', async({page, server}) => { - const handle = await page.waitForSelector('non-existing', { waitFor: 'hidden' }); + const handle = await page.waitForSelector('non-existing', { visibility: 'hidden' }); expect(handle).toBe(null); }); it('should respect timeout', async({page, server}) => { @@ -384,7 +384,7 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO it('should have an error message specifically for awaiting an element to be hidden', async({page, server}) => { await page.setContent(`
`); let error = null; - await page.waitForSelector('div', { waitFor: 'hidden', timeout: 10 }).catch(e => error = e); + await page.waitForSelector('div', { visibility: 'hidden', timeout: 10 }).catch(e => error = e); expect(error).toBeTruthy(); expect(error.message).toContain('waiting for selector "[hidden] div" failed: timeout'); }); @@ -407,33 +407,24 @@ module.exports.describe = function({testRunner, expect, product, playwright, FFO await page.waitForSelector('.zombo', { timeout: 10 }).catch(e => error = e); expect(error.stack).toContain('waittask.spec.js'); }); - it('should throw for waitFor nowait', async({page, server}) => { - let error; - try { - await page.waitForSelector('non-existing-element', { waitFor: 'nowait' }); - } catch (e) { - error = e; - } - expect(error.message).toBe('waitForSelector does not support "nowait"'); - }); it('should throw for unknown waitFor option', async({page, server}) => { await page.setContent('
test
'); - const error = await page.waitForSelector('section', { waitFor: 'foo' }).catch(e => e); + const error = await page.waitForSelector('section', { visibility: 'foo' }).catch(e => e); expect(error.message).toContain('Unsupported waitFor option'); }); it('should throw for numeric waitFor option', async({page, server}) => { await page.setContent('
test
'); - const error = await page.waitForSelector('section', { waitFor: 123 }).catch(e => e); + const error = await page.waitForSelector('section', { visibility: 123 }).catch(e => e); expect(error.message).toContain('Unsupported waitFor option'); }); it('should throw for true waitFor option', async({page, server}) => { await page.setContent('
test
'); - const error = await page.waitForSelector('section', { waitFor: true }).catch(e => e); + const error = await page.waitForSelector('section', { visibility: true }).catch(e => e); expect(error.message).toContain('Unsupported waitFor option'); }); it('should throw for false waitFor option', async({page, server}) => { await page.setContent('
test
'); - const error = await page.waitForSelector('section', { waitFor: false }).catch(e => e); + const error = await page.waitForSelector('section', { visibility: false }).catch(e => e); expect(error.message).toContain('Unsupported waitFor option'); }); it('should support >> selector syntax', async({page, server}) => { diff --git a/utils/doclint/check_public_api/Documentation.js b/utils/doclint/check_public_api/Documentation.js index 095c614902..87af1ed914 100644 --- a/utils/doclint/check_public_api/Documentation.js +++ b/utils/doclint/check_public_api/Documentation.js @@ -72,6 +72,50 @@ Documentation.Class = class { } } } + + validateOrder(errors) { + const members = this.membersArray; + // Events should go first. + let eventIndex = 0; + for (; eventIndex < members.length && members[eventIndex].kind === 'event'; ++eventIndex); + for (; eventIndex < members.length && members[eventIndex].kind !== 'event'; ++eventIndex); + if (eventIndex < members.length) + errors.push(`Events should go first. Event '${members[eventIndex].name}' in class ${cls.name} breaks order`); + + // Constructor should be right after events and before all other members. + const constructorIndex = members.findIndex(member => member.kind === 'method' && member.name === 'constructor'); + if (constructorIndex > 0 && members[constructorIndex - 1].kind !== 'event') + errors.push(`Constructor of ${cls.name} should go before other methods`); + + // Events should be sorted alphabetically. + for (let i = 0; i < members.length - 1; ++i) { + const member1 = this.membersArray[i]; + const member2 = this.membersArray[i + 1]; + if (member1.kind !== 'event' || member2.kind !== 'event') + continue; + if (member1.name > member2.name) + errors.push(`Event '${member1.name}' in class ${this.name} breaks alphabetic ordering of events`); + } + + // All other members should be sorted alphabetically. + for (let i = 0; i < members.length - 1; ++i) { + const member1 = this.membersArray[i]; + const member2 = this.membersArray[i + 1]; + if (member1.kind === 'event' || member2.kind === 'event') + continue; + if (member1.kind === 'method' && member1.name === 'constructor') + continue; + if (member1.name > member2.name) { + let memberName1 = `${this.name}.${member1.name}`; + if (member1.kind === 'method') + memberName1 += '()'; + let memberName2 = `${this.name}.${member2.name}`; + if (member2.kind === 'method') + memberName2 += '()'; + errors.push(`Bad alphabetic ordering of ${this.name} members: ${memberName1} should go after ${memberName2}`); + } + } + } }; Documentation.Member = class { diff --git a/utils/doclint/check_public_api/MDBuilder.js b/utils/doclint/check_public_api/MDBuilder.js index 0bc5a31768..f1910c9a3b 100644 --- a/utils/doclint/check_public_api/MDBuilder.js +++ b/utils/doclint/check_public_api/MDBuilder.js @@ -300,6 +300,8 @@ module.exports = async function(page, sources) { // Push base class documentation to derived classes. for (const [name, clazz] of documentation.classes.entries()) { + clazz.validateOrder(errors); + if (!clazz.extends || clazz.extends === 'EventEmitter' || clazz.extends === 'Error') continue; const superClass = documentation.classes.get(clazz.extends); @@ -311,6 +313,7 @@ module.exports = async function(page, sources) { if (superClass.members.has(memberName)) errors.push(`Member documentation overrides base: ${name}.${memberName} over ${clazz.extends}.${memberName}`); } + clazz.membersArray = [...clazz.membersArray, ...superClass.membersArray]; clazz.index(); } diff --git a/utils/doclint/check_public_api/index.js b/utils/doclint/check_public_api/index.js index 6aa3a783c8..54138bf5c3 100644 --- a/utils/doclint/check_public_api/index.js +++ b/utils/doclint/check_public_api/index.js @@ -45,7 +45,6 @@ module.exports = async function lint(page, mdSources, jsSources) { const mdErrors = mdResult.errors; mdErrors.push(...compareDocumentations(mdDocumentation, jsDocumentation)); mdErrors.push(...checkDuplicates(mdDocumentation)); - mdErrors.push(...checkSorting(mdDocumentation)); // Push all errors with proper prefixes const errors = jsErrors.map(error => '[JavaScript] ' + error); @@ -53,59 +52,6 @@ module.exports = async function lint(page, mdSources, jsSources) { return errors.map(error => Message.error(error)); }; -/** - * @param {!Documentation} doc - * @return {!Array} - */ -function checkSorting(doc) { - const errors = []; - for (const cls of doc.classesArray) { - const members = cls.membersArray; - - // Events should go first. - let eventIndex = 0; - for (; eventIndex < members.length && members[eventIndex].kind === 'event'; ++eventIndex); - for (; eventIndex < members.length && members[eventIndex].kind !== 'event'; ++eventIndex); - if (eventIndex < members.length) - errors.push(`Events should go first. Event '${members[eventIndex].name}' in class ${cls.name} breaks order`); - - // Constructor should be right after events and before all other members. - const constructorIndex = members.findIndex(member => member.kind === 'method' && member.name === 'constructor'); - if (constructorIndex > 0 && members[constructorIndex - 1].kind !== 'event') - errors.push(`Constructor of ${cls.name} should go before other methods`); - - // Events should be sorted alphabetically. - for (let i = 0; i < members.length - 1; ++i) { - const member1 = cls.membersArray[i]; - const member2 = cls.membersArray[i + 1]; - if (member1.kind !== 'event' || member2.kind !== 'event') - continue; - if (member1.name > member2.name) - errors.push(`Event '${member1.name}' in class ${cls.name} breaks alphabetic ordering of events`); - } - - // All other members should be sorted alphabetically. - for (let i = 0; i < members.length - 1; ++i) { - const member1 = cls.membersArray[i]; - const member2 = cls.membersArray[i + 1]; - if (member1.kind === 'event' || member2.kind === 'event') - continue; - if (member1.kind === 'method' && member1.name === 'constructor') - continue; - if (member1.name > member2.name) { - let memberName1 = `${cls.name}.${member1.name}`; - if (member1.kind === 'method') - memberName1 += '()'; - let memberName2 = `${cls.name}.${member2.name}`; - if (member2.kind === 'method') - memberName2 += '()'; - errors.push(`Bad alphabetic ordering of ${cls.name} members: ${memberName1} should go after ${memberName2}`); - } - } - } - return errors; -} - /** * @param {!Array} jsSources * @param {!Documentation} jsDocumentation