refactor(linter/plugins): remove | undefined from optional function params types
#15885
Conversation
github-actions
bot
added
A-linter
graphite-app
bot
changed the base branch from
11-19-docs_linter_plugins_fix_jsdoc_comment
to
graphite-base/15885
Merge activity
|
… params types (#15885) `| undefined` in optional function params (e.g. `param?: string | undefined`) is pointless. Remove them.
graphite-app
bot
force-pushed
the
11-19-refactor_linter_plugins_remove_undefined_from_optional_function_params_types
branch
from
f870a1a to
b2de90b
Compare
graphite-app
bot
deleted the
11-19-refactor_linter_plugins_remove_undefined_from_optional_function_params_types
branch
| beforeCount?: number | null | undefined, | ||
| afterCount?: number | null | undefined, | ||
| ): string { | ||
| getText(node?: Ranged | null, beforeCount?: number | null, afterCount?: number | null): string { |
There was a problem hiding this comment.
I think we should copy eslint's API and remove | null here.
There was a problem hiding this comment.
Hmm. If ESLint doesn't accept null, then yes I guess we shouldn't either, so that plugins written and tested in Oxlint remain compatible with ESLint. But in a lot of cases, ESLint uses !option to check for defined/undefined, so is very liberal indeed. I think ideally we don't want to do that as truthiness checks are somewhat expensive compared to == null (I believe).
There was a problem hiding this comment.
I think ===undefined is probably faster than ==null i would guess?
There was a problem hiding this comment.
Yes. We only use == null where we want to catch either. Wherever possible, we use === undefined or === null.
But if it was me designing a lib, I'd accept either null or undefined to mean "nothing" in all external APIs, and conform the input so we only use null internally.
… params types (#15885) `| undefined` in optional function params (e.g. `param?: string | undefined`) is pointless. Remove them.
| undefinedin optional function params (e.g.param?: string | undefined) is pointless. Remove them.