Strict TypeScript cleanup #2244
Conversation
| const objStore = txn.objectStore(storeName) | ||
| const objStore = txn.objectStore(storeName); | ||
| // TODO(philipwalton): Fix this underlying TS2684 error. | ||
| // @ts-ignore |
There was a problem hiding this comment.
This is the biggest issue, and I just punted on it as I don't have any idea how to resolve it. @philipwalton, if you can look into it as part of either another PR or push an update to this PR, that would be great.
| @@ -19,7 +19,8 @@ import './_version.js'; | |||
| * @alias workbox.precaching.cleanupOutdatedCaches | |||
| */ | |||
| export const cleanupOutdatedCaches = () => { | |||
| addEventListener('activate', (event: ExtendableEvent) => { | |||
| // See https://github.com/Microsoft/TypeScript/issues/28357#issuecomment-436484705 | |||
There was a problem hiding this comment.
There are a lot of similar changes to this one, due to TypeScript being unhappy about the listener function taking a parameter of type ExtendableEvent.
Declaring these functions as being of type EventListener seems fairly "safe", but perhaps the more official route would be to override the addEventListener definition so that it supports a listener function that takes in an ExtendableEvent parameter, instead of just an Event parameter.
| @@ -128,11 +130,10 @@ class Router { | |||
|
|
|||
| // If a MessageChannel was used, reply to the message on success. | |||
| if (event.ports && event.ports[0]) { | |||
| await requestPromises; | |||
| event.ports[0].postMessage(true); | |||
| requestPromises.then(() => event.ports[0].postMessage(true)); | |||
There was a problem hiding this comment.
Declaring the listener function as being an EventListener wouldn't work if it was async, since EventListeners aren't supposed to return Promise<void>.
So I just took the one await and converted it into a then(), and dropped the async from the listener function declaration.
| @@ -27,7 +27,7 @@ export class WorkboxEventTarget { | |||
| */ | |||
| addEventListener<K extends keyof WorkboxEventMap>(type: K, listener: (event: WorkboxEventMap[K]) => any) { | |||
| const foo = this._getEventListenersByType(type) | |||
| foo.add(listener); | |||
| foo.add(listener as ListenerCallback); | |||
There was a problem hiding this comment.
This and the other place where I declare listener as being a ListenerCallback might be a little sketchy, so your thoughts are appreciated.
The underlying error is because some varieties of listener might have a data property (the one that wraps a message event), and TypeScript isn't happy about that.
There might be a better remediation here, like changing the _getEventListenersByType() definition so that it returns a Set with different signature.
PR-Bot Size PluginChanged File Sizes
New FilesNo new files have been added. All File SizesView Table
Workbox Aggregate Size Plugin3.51KB gzip'ed (23% of limit) |
R: @philipwalton
Fixes #2242
This cleans up a number of problems that were found by switching to using
strict: truein the TypeScript compiler options.There are some... questionable?... casts going on, so I'll address them via comments on the specific lines to see whether there are better approaches.