chore(gatsby-cli): Convert reporter to TS #22252
Conversation
MichaelDeBoey
changed the title
| format: string, | ||
| ...args: unknown[] | ||
| ): IStructuredError | IStructuredError[] => | ||
| reporter.error(util.format(format, ...args)) | ||
|
|
||
| module.exports = reporter |
There was a problem hiding this comment.
we should be changing this to a named export
There was a problem hiding this comment.
I left the export the same because it's a breaking change, and is used in so many places
|
|
||
| /** | ||
| * Reporter module. | ||
| * @module reporter | ||
| */ | ||
| const reporter: Reporter = { | ||
| class Reporter { |
There was a problem hiding this comment.
What is the benefit of making this a class over an object?
| }) | ||
|
|
||
| this.interuptActivities() | ||
| } |
There was a problem hiding this comment.
I am happy to accept this PR if you take these 2 functions out of the class. It's likely not a big deal, but we are growing the api surface of this module by putting these 2 functions on the class.
I loved your intention of removing top level side effects, but technically you haven't 😢, you just moved them. Since we instantiate this class in this file, we run the constructor on import. So it's all the same. I'm happy with that change though because it feels confined, but adding the interuptActivities and prematureEnd to the class grows the api surface and that runs the risk of someone calling those methods because they exist without it being the correct thing to do.
|
Closing in favor of #22869 💜 |
Description
Types the main
reporterfilesI have switcher
Reporterfrom being a plain object to being a class with a singleton export.Part of #21995