Suggestion: Refactor `report` API to take an `options` object

[antross]

This API has grown to contain a number of optional parameters which can lead to awkward calls like:

await context.report(resource, null, 'Resource has no content.', undefined, problemLocation);

Current definition

public async report(resource: string, element: IAsyncHTMLElement | null, message: string, content?: string, location?: ProblemLocation, severity?: Severity, codeSnippet?: string): Promise<void>

The only two pieces that are truly required are the first and third parameters (resource and message).

Proposed change

I propose either replacing or providing an alternate version of this API which takes an options object instead (depending on whether we're willing to take this as a breaking change):

type ReportOptions = {
    codeSnippet?: string;
    content?: string;
    element?: IAsyncHTMLElement | null;
    location?: ProblemLocation;
    severity?: Severity;
};

public async report(resource: string, message: string, options?: ReportOptions): Promise<void>

[molant]

Could be a duplicate of https://github.com/webhintio/hint/issues/258. I'm ok closing that one in favor of this. @antross ?

[antross]

I'm ok closing that one in favor of this

Sure, looks like we were thinking along the same lines. :)

Looking there I also see we apparently need to document this API, which we should cover in whatever PR makes this change.

@molant what are your thoughts on replacing the existing API entirely (making this a breaking change)?

[molant]

I'd like to break as many things as we can as the same time so we don't have a major version every other week. Maybe now is the time with the current pending PRs? I'd like to get the debugging protocol changes before any breaking change so we can publish it sooner rather than later.

[antross]

I'd like to break as many things as we can as the same time so we don't have a major version every other week.

Agreed, and yes now would probably be a good time since I'm sitting on a breaking change PR already. I'll add this to my list for today.

[molant]

great! Can you write here your proposal for the API before doing any major change so we can discuss it?

[antross]

Sure, are you looking for something other than what was listed at the end of my opening comment?

[molant]

Ups, missed that part 😅 Maybe an updated version in case you find something else while looking at all the places where we use it, although I don't remember any weird usage of this. Otherwise your proposal LGTM. @webhintio/core what do you think of @antross proposed changes?

[sarvaje]

I like the idea!