-
Notifications
You must be signed in to change notification settings - Fork 50
ref: Enable explicit-function-return-type
eslint rule
#742
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
Conversation
hmm looks like I broke some tests 😕 -- setting to draft for now |
6c7c1ea
to
fb03f50
Compare
} | ||
// TODO(v4): Don't export this from the package but keep the utils version | ||
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type | ||
export const getBuildInformation = actualGetBuildInformation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We had 1:1 the same implementation in the actualGetBuildInformation
util function, so I just removed the duplication (but kept the top level export deprecated).
/** | ||
* Simplified `renderChunk` hook type from Rollup. | ||
* We can't reference the type directly because the Vite plugin complains | ||
* about type mismatches | ||
*/ | ||
type RenderChunkHook = ( | ||
code: string, | ||
chunk: { | ||
fileName: string; | ||
} | ||
) => { | ||
code: string; | ||
map: SourceMap; | ||
} | null; | ||
|
||
export function createRollupDebugIdInjectionHooks(): { | ||
renderChunk: RenderChunkHook; | ||
} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment explains this but maybe just to add: I can't use UnpluginOptions["renderChunk"];
because renderChunk
is not a hook known to Unplugin. We manually pass in this hook in a plugin specifically for Rollup and Vite.
writeBundle: ( | ||
outputOptions: { dir?: string; file?: string }, | ||
bundle: { [fileName: string]: unknown } | ||
) => Promise<void>; | ||
} { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same for writeBundle
as for renderChunk
(see comment above)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for improving this!
This PR turns on the
explicit-function-return-type
eslint rule. I noticed in #741 that we tended to export public API with implicit function return types (and sometimes even declared types dynamically from implicit function return types). This has high potential to introduce accidental breaking changes. While I rewrote the biggest offenders in #741, this PR now turns on an eslint rule to enforce explicit function return types.As a result of the eslint rule, I had to turn add some return types but most of them are very straight forward. I'll leave comments on things that need special attention.