New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit typed events? #123

Closed
molant opened this Issue Apr 17, 2017 · 3 comments

Comments

Projects
3 participants
@molant
Member

molant commented Apr 17, 2017

Right now we are using emitAsync and trying to type the data we are sending through this method. The problem is that sometimes we forget (or will forget).
Should we create new events (emitElement, emitFetchStart, etc.) that will force the parameter to be of the right type?
pros:
No need to remember to type the object to be sure it is of the right type.
cons:

  • Need to implement a method for each event.
  • We might run into issues when testing because the types are different.

@alrra, @sarvaje, thoughts?

@molant molant added this to Miscelaneous in v1 Apr 17, 2017

@molant

This comment has been minimized.

Member

molant commented Jun 30, 2017

Something similar is being proposed to webpack in this Twitter thread.

Pinging @sonarwhal/contributors to get their thoughts. This will be breaking so better do it before v1

@alrra alrra referenced this issue Mar 1, 2018

Closed

Refactor config, parsers, formatters and types #844

4 of 4 tasks complete
@alrra

This comment has been minimized.

Member

alrra commented Mar 1, 2018

Closing as per #844 (comment).

@molant

This comment has been minimized.

Member

molant commented Mar 19, 2018

Reopening because of this post by @brterlson

We can probably do it in a not very complicated way :)

@molant molant reopened this Mar 19, 2018

@antross antross self-assigned this Oct 10, 2018

@molant molant added this to In progress in October '18 🍁🍂 Oct 11, 2018

antross added a commit to antross/hint that referenced this issue Oct 13, 2018

Breaking: Use typed event registration and dispatch
Fix webhintio#123. No change is required for hints which consume only the
core set of events (`fetch::*`, `element::*`, `scan::*`, `traverse::*`).

Hints which consume parser-specific events such as
`parse::manifest::end` now need to import the events for that parser
and apply them to their `HintContext`:

```typescript
import { ManifestEvents } from `@hint/parser-manifest`;

public constructor(context: HintContext<ManifestEvents>) {
    ...
    context.on('parse::manifest::end', checkManifest);
}
```

Similarly, new `Parser`s need to explicitly declare and export added
events so they can be imported and consumed elsewhere:

```typescript
import { Events } from `hint/dist/src/lib/types`;
...
export type ManifestEvents = Events & {
    'fetch:🔚:manifest': FetchEnd;
    'fetch::error::manifest': FetchError;
    'fetch::start::manifest': FetchStart;
    'parse::manifest::end': ManifestParsed;
    'parse::manifest::error::schema': ManifestInvalidSchema;
    'parse::manifest::error::json': ManifestInvalidJSON;
};
```

antross added a commit to antross/hint that referenced this issue Oct 13, 2018

Fix: Use typed event registration
Refs webhintio#123. Fixes affected hints and connectors to work with changes
introduced by ef41732.

@molant molant moved this from In progress to Needs review in October '18 🍁🍂 Oct 18, 2018

antross added a commit to antross/hint that referenced this issue Nov 1, 2018

Breaking: Use typed event registration and dispatch
Fix webhintio#123. No change is required for hints which consume only the
core set of events (`fetch::*`, `element::*`, `scan::*`, `traverse::*`).

Hints which consume parser-specific events such as
`parse::manifest::end` now need to import the events for that parser
and apply them to their `HintContext`:

```typescript
import { ManifestEvents } from `@hint/parser-manifest`;

public constructor(context: HintContext<ManifestEvents>) {
    ...
    context.on('parse::manifest::end', checkManifest);
}
```

Similarly, new `Parser`s need to explicitly declare and export added
events so they can be imported and consumed elsewhere:

```typescript
import { Events } from `hint/dist/src/lib/types`;
...
export type ManifestEvents = Events & {
    'fetch:🔚:manifest': FetchEnd;
    'fetch::error::manifest': FetchError;
    'fetch::start::manifest': FetchStart;
    'parse::manifest::end': ManifestParsed;
    'parse::manifest::error::schema': ManifestInvalidSchema;
    'parse::manifest::error::json': ManifestInvalidJSON;
};
```

antross added a commit to antross/hint that referenced this issue Nov 1, 2018

Fix: Use typed event registration
Refs webhintio#123. Fixes affected hints and connectors to work with changes
introduced by ef41732.

@alrra alrra closed this in d181168 Nov 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment