Skip to content

Commit

Permalink
Breaking: Remove IAsync* and use HTMLDocument and HTMLElement
Browse files Browse the repository at this point in the history
  • Loading branch information
sarvaje authored and antross committed Mar 8, 2019
1 parent d8490b3 commit 889327d
Show file tree
Hide file tree
Showing 21 changed files with 52 additions and 775 deletions.
12 changes: 7 additions & 5 deletions packages/hint/docs/contributor-guide/getting-started/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { Event, Events } from 'hint/dist/src/lib/types/events';
...
export type StyleParse = Event & {
ast: Root;
code: string;
element: HTMLElement | null;
};

export type StyleEvents = Events & {
Expand Down Expand Up @@ -58,7 +60,7 @@ type ElementFound = {
/** The URI of the resource firing this event. */
resource: string;
/** The visited element. */
element: IAsyncHTMLElement;
element: HTMLElement;
}
```
Expand All @@ -72,7 +74,7 @@ Event is emitted **when** the content of a `resource` (`js`, `css`,
```ts
type FetchEnd {
/** The element that initiated the request. */
element: IAsyncHTMLElement;
element: HTMLElement;
/** The URL of the target */
resource: string;
/** The request made to fetch the target. */
Expand All @@ -94,7 +96,7 @@ type FetchError {
/** The URL of the target. */
resource: string;
/** The element that initiated the request. */
element: IAsyncHTMLElement;
element: HTMLElement;
/** The error found. */
error: any;
/** The redirects performed for the url. */
Expand Down Expand Up @@ -161,7 +163,7 @@ is to be traversed.
```ts
type TraverseDown {
/** The parent element to be traversed. */
element: IAsyncHTMLElement;
element: HTMLElement;
/** The URL of the target. */
resource: string;
}
Expand Down Expand Up @@ -206,7 +208,7 @@ node that was traversed.
```ts
type TraverseUp {
/** The parent element that was traversed. */
element: IAsyncHTMLElement;
element: HTMLElement;
/** The URL of the target. */
resource: string;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ validateFooter function. As shown in the code below, we have access to the
footer element in the page and use that for our check - if the HTML doesn’t
include the target string, we simply file a report by calling `context.report`
with the *resource* (URL), the *element* (footer), and the *error message*.
Note that `context.report` is an asynchronous method, so always use `await`.

```ts
export default class FooterHint implements IHint {
Expand All @@ -111,14 +110,14 @@ export default class FooterHint implements IHint {
const stringToBeIncluded = `(c) webhint`;
const validateFooter = async (elementFound: ElementFound) => {
const { element, resource } = elementFound;
const footerHTML = await element.outerHTML();
const footerHTML = element.outerHTML();

debug(`Validating hint validate-footer`);

if (!footerHTML.includes(stringToBeIncluded)) {
const message = `"${stringToBeIncluded}" is not included in the footer.`;

await context.report(resource, message, { element });
context.report(resource, message, { element });
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export default class ScriptSemiColonHint implements IHint {
});

for (const result of results) {
await context.report(scriptParse.resource, null, result.message);
context.report(scriptParse.resource, null, result.message);
}
};

Expand Down
48 changes: 3 additions & 45 deletions packages/hint/docs/contributor-guide/how-to/connector.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,8 @@ this order:
or if there are, the connector has waited a reasonable amount of
time. The traversing of the DOM is [depth-first][depth-first search],
sending:
* [`element::<element-type>`][events element]
when visiting a node (see [IASyncHTML](#iasynchtml) for more
information,
* [`traverse::down`][events traversedown] when going deeper
in the DOM,
* [`element::<element-type>`][events element] when visiting a node,
* [`traverse::down`][events traversedown] when going deeper in the DOM,
* [`traverse::up`][events traverseup] when going up.
1. [`traverse::end`][events traverseend]
1. The final event is [`scan::end`][events scanend].
Expand All @@ -74,49 +71,10 @@ export interface IConnector {
/** Evaluates the given JavaScript `code` asynchronously in the target. */
evaluate(code: string): Promise<any>;
/** Finds all the nodes that match the given query. */
querySelectorAll(query: string): Promise<IAsyncHTMLElement[]>
querySelectorAll(query: string): HTMLElement[]
}
```

### IASyncHTML

`IAsyncHTML` is an abstraction on top of the connector’s DOM. The reason
is that some connectors can access the DOM synchronously (like `jsdom`)
and some others don’t (like those that rely on a debugging protocol).
We decided to create an asynchronous abstraction so the different parts
that might need access to the DOM know how to use. `IAsyncHTML` is
composed two interfaces: `IAsyncHTMLElement` and `IAsyncHTMLDocument`.
Not all the `HTMLElement` or `HTMLDocument` properties are implemented:

```ts
/** A wrapper of an HTMLElement that gives access to the required resources
* asynchronously to be compatible with all connectors */
export interface IAsyncHTMLElement {
/** The attributes of the element */
readonly attributes: Array<{ name: string, value: string }> | NamedNodeMap;
/** Returns the value for a given attribute */
getAttribute(attribute: string): string;
/** Checks if two AsyncHTMLElements are the same */
isSame(element: IAsyncHTMLElement): boolean;
/** Returns the outerHTML of the element */
outerHTML(): Promise<string>;
/** Returns the document where the element lives */
readonly ownerDocument: IAsyncHTMLDocument;
/** The nodeName of the element */
readonly nodeName: string
}

export interface IAsyncHTMLDocument {
/** A wrapper around querySelectorAll that returns an Array of AsyncHTMLElements instead of a NodeList */
querySelectorAll(selector: string): Promise<IAsyncHTMLElement[]>
/** The HTML of the page as returned by document.children[0].outerHTML or similar */
pageHTML(): Promise<string>;
}
```

When traversing the DOM, the `element` in the `event` needs to be an
`IAsyncHTMLElement`.

## How to test a "full" connector

To make sure your connector is a "full" connector, it has to pass the
Expand Down
5 changes: 2 additions & 3 deletions packages/hint/docs/contributor-guide/how-to/hint.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,15 +93,14 @@ with the website and report errors.
To report an error, the hint has to do the following:

```ts
await context.report(resource, message, { element: element });
context.report(resource, message, { element: element });
```

* `context.report()` is an asynchronous method, you should always `await`.
* `resource` is the URL of what is being analyzed (HTML, JS, CSS, manifest,
etc.)
* `message` is the text to show to the user about the problem.
* `options` is an (optional) object that can contain the following:
* `element` is an optional `IAsyncHTMLElement` where the issue was found
* `element` is an optional `HTMLElement` where the issue was found
(used to get a `ProblemLocation` if one was not provided). For example,
if an image is missing an `alt` attribute, this can be the `img` element.
* `codeSnippet` is a string of source code to display (defaults to the
Expand Down
5 changes: 2 additions & 3 deletions packages/hint/src/lib/cli/analyze.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import * as path from 'path';

import * as async from 'async';
import { default as ora, Ora } from 'ora';
const boxen = require('boxen'); // `require` used because `boxen` exports a function

import * as chalk from 'chalk';
import * as isCI from 'is-ci';
import { EventAndListener } from 'eventemitter2';
Expand All @@ -25,6 +23,7 @@ import * as insights from '../utils/app-insights';
import { FormatterOptions, IFormatter } from '../types/formatters';
import loadHintPackage from '../utils/packages/load-hint-package';

const boxen = require('boxen'); // `require` used because `boxen` exports a function
const each = promisify(async.each);
const debug: debug.IDebugger = d(__filename);
const configStoreKey: string = 'run';
Expand Down Expand Up @@ -196,7 +195,7 @@ const getUserConfig = (actions?: CLIOptions): UserConfig | null => {
}
};

const messages: {[name: string]: string} = {
const messages: { [name: string]: string } = {
'fetch::end': '%url% downloaded',
'fetch::start': 'Downloading %url%',
'scan::end': 'Finishing...',
Expand Down
9 changes: 5 additions & 4 deletions packages/hint/src/lib/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
HintConfig,
HintResources,
HttpHeaders,
IAsyncHTMLElement,
HTMLElement,
IConnector,
IConnectorConstructor,
IFetchOptions,
Expand All @@ -41,6 +41,7 @@ import { HintContext } from './hint-context';
import { HintScope } from './enums/hint-scope';
import { Configuration } from './config';
import { Category } from './enums/category';
import { HTMLDocument } from './types/html';

const debug: debug.IDebugger = d(__filename);

Expand All @@ -62,12 +63,12 @@ export class Engine<E extends Events = Events> extends EventEmitter {
private _timeout: number = 60000;

/** The DOM of the loaded page. */
public get pageDOM(): object | undefined {
public get pageDOM(): HTMLDocument | undefined {
return this.connector.dom;
}

/** The HTML of the loaded page. */
public get pageContent(): Promise<string> | undefined {
public get pageContent(): string | undefined {
return this.connector.html;
}

Expand Down Expand Up @@ -323,7 +324,7 @@ export class Engine<E extends Events = Events> extends EventEmitter {
return this.messages;
}

public querySelectorAll(selector: string): Promise<IAsyncHTMLElement[]> {
public querySelectorAll(selector: string): HTMLElement[] {
return this.connector.querySelectorAll(selector);
}

Expand Down
26 changes: 10 additions & 16 deletions packages/hint/src/lib/hint-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,22 +10,21 @@ import { Engine } from './engine';
import {
Events,
HintMetadata,
IAsyncHTMLElement,
HTMLElement,
NetworkData,
ProblemLocation,
Severity,
StringKeyOf
} from './types';
import { findInElement, findProblemLocation } from './utils/location-helpers';
import { Category } from './enums/category';

export type ReportOptions = {
/** The source code to display (defaults to the `outerHTML` of `element`). */
codeSnippet?: string;
/** The text within `element` where the issue was found (used to refine a `ProblemLocation`). */
content?: string;
/** The `IAsyncHTMLElement` where the issue was found (used to get a `ProblemLocation`). */
element?: IAsyncHTMLElement | null;
/** The `HTMLElement` where the issue was found (used to get a `ProblemLocation`). */
element?: HTMLElement | null;
/** The `ProblemLocation` where the issue was found. */
location?: ProblemLocation | null;
/** The `Severity` to report the issue as (overrides default settings for a hint). */
Expand Down Expand Up @@ -98,29 +97,24 @@ export class HintContext<E extends Events = Events> {
return this.engine.fetchContent(target, headers);
}

public querySelectorAll(selector: string): Promise<IAsyncHTMLElement[]> {
public querySelectorAll(selector: string): HTMLElement[] {
return this.engine.querySelectorAll(selector);
}

/** Finds the exact location of the given content in the HTML that represents the `element`. */
public findInElement(element: IAsyncHTMLElement, content: string): Promise<ProblemLocation> {
return findInElement(element, content);
}

/** Finds the approximative location in the page's HTML for a match in an element. */
public findProblemLocation(element: IAsyncHTMLElement, content?: string): Promise<ProblemLocation> {
return findProblemLocation(element, { column: 0, line: 0 }, content);
public findProblemLocation(element: HTMLElement): ProblemLocation {
return element.getLocation();
}

/** Reports a problem with the resource. */
public async report(resource: string, message: string, options: ReportOptions = {}): Promise<void> {
const { codeSnippet, content, element, severity } = options;
public report(resource: string, message: string, options: ReportOptions = {}) {
const { codeSnippet, element, severity } = options;
let position: ProblemLocation | null = options.location || null;
let sourceCode: string | null = null;

if (element) {
position = await findProblemLocation(element, { column: 0, line: 0 }, content);
sourceCode = (await element.outerHTML()).replace(/[\t]/g, ' ');
position = this.findProblemLocation(element);
sourceCode = element.outerHTML().replace(/[\t]/g, ' ');
}

/*
Expand Down
2 changes: 1 addition & 1 deletion packages/hint/src/lib/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { IParserConstructor } from './types/parser';
import { IHintConstructor } from './types/hints';
import { Severity } from './types/problems';

export * from './types/async-html';
export * from './types/html';
export * from './types/connector';
export * from './types/events';
export * from './types/formatters';
Expand Down
50 changes: 0 additions & 50 deletions packages/hint/src/lib/types/async-html.ts

This file was deleted.

8 changes: 4 additions & 4 deletions packages/hint/src/lib/types/connector.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as url from 'url';

import { IAsyncHTMLElement } from './async-html';
import { HTMLElement, HTMLDocument } from './html';
import { HttpHeaders, NetworkData } from './network';
import { Engine } from '../engine';

Expand All @@ -11,9 +11,9 @@ export interface IConnectorConstructor {
/** A connector to be used by hint */
export interface IConnector {
/** The original DOM of the resource collected. */
dom?: object;
dom?: HTMLDocument;
/** The original HTML of the resource collected. */
html?: Promise<string>;
html?: string;
/** The headers from the response if applicable. */
headers?: HttpHeaders;
/** Collects all the information for the given target. */
Expand All @@ -25,7 +25,7 @@ export interface IConnector {
/** Evaluates the given JavaScript `code` asynchronously in the target. */
evaluate(code: string): Promise<any>;
/** Finds all the nodes that match the given query. */
querySelectorAll(query: string): Promise<IAsyncHTMLElement[]>;
querySelectorAll(query: string): HTMLElement[];
}

/** Additional detail for calls to `connect` and `fetchContent` on `IConnector`. */
Expand Down
Loading

0 comments on commit 889327d

Please sign in to comment.