-
-
Notifications
You must be signed in to change notification settings - Fork 927
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
Add flow comments annotation to function signatures #2319
Conversation
lib/assignDisabledRanges.js
Outdated
const enableCommand/*: string*/ = COMMAND_PREFIX + "enable" | ||
const disableLineCommand/*: string*/ = COMMAND_PREFIX + "disable-line" | ||
const disableNextLineCommand/*: string*/ = COMMAND_PREFIX + "disable-next-line" | ||
const ALL_RULES/*: string*/ = "all" |
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.
There is no need to annotate constants, since Flow can understand them just fine.
lib/assignDisabledRanges.js
Outdated
@@ -20,7 +20,7 @@ const ALL_RULES = "all" | |||
module.exports = function ( | |||
root/*: Object*/, | |||
result/*: Object*/ | |||
) { | |||
)/*: Object*/ { |
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.
Does this always return an object? If so, what object? Is there a more precise annotation we can give?
@ivanzusko As discussed in that other PR, let's remove all of the annotations on variables and just stick with those on the function signatures. Can you add any more specificity than |
…ty to returning object;
@davidtheclark , hope I made a correct annotation to returning object... |
lib/assignDisabledRanges.js
Outdated
// Run it like a plugin ... | ||
module.exports = function ( | ||
root/*: Object*/, | ||
result/*: Object*/ | ||
) { | ||
)/*: returningResult*/ { |
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 return value here is a postcss$result
, right?
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.
@davidtheclark do you mean I have to rename this type?
There are just several points:
- I did not find
postcss$result
in declaration files, that's why I assumed that you've meant renaming - I've run tests and found out that returning object is much bigger than I described. Should I expand the type annotation of the returning object?
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.
@davidtheclark , I got it!
lib/assignDisabledRanges.js
Outdated
@@ -121,7 +130,7 @@ module.exports = function ( | |||
} | |||
|
|||
function checkComment(comment/*: postcss$comment*/) { | |||
const text = comment.text | |||
const text/*: string*/ = comment.text |
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.
No need for this.
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.
fixed
lib/assignDisabledRanges.js
Outdated
if (_.isEmpty(rules)) { | ||
return [ALL_RULES] | ||
} | ||
return rules | ||
} | ||
|
||
function startDisabledRange(line/*: number*/, ruleName/*: string*/) { | ||
const rangeObj = { start: line } | ||
const rangeObj/*: Object*/ = { start: line } |
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.
No need to annotate this.
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.
fixed
lib/assignDisabledRanges.js
Outdated
@@ -144,15 +153,15 @@ module.exports = function ( | |||
command/*: string*/, | |||
fullText/*: string*/ | |||
)/*: Array<string>*/ { | |||
const rules = _.compact(fullText.slice(command.length).split(",")).map(r => r.trim()) | |||
const rules/*: Array<string>*/ = _.compact(fullText.slice(command.length).split(",")).map(r => r.trim()) |
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.
No need to annotate this.
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.
fixed
Thanks @ivanzusko! |
@davidtheclark thank you :) |
* master: Update CHANGELOG.md Add flow comments annotation to function signatures (#2319) Update CHANGELOG.md Fix custom properties sets followed by comments in no-extra-semicolons (#2396) Fixed: `value-keyword-case` is now ignore counter, counters and attr fucntions. Fix typos (#2412) Fixed: `value-keyword-case` is now ignore counter-reset property. chore(package): update flow-bin to version 0.41.0 chore(package): update eslint to version 3.17.0 Fix the example for Custom Message in docs (#2409) Update CHANGELOG.md Update CHANGELOG.md Add at-rule-semicolon-space-before rule (#2388) Update CHANGELOG.md Print out filenames/globs when files do not exist (#2328) Tests: improved tests for `no-missing-end-of-source-newline` rule. (#2384) Update stylelint-disable-reason changelog entry (#2393) Add min rules chore(package): update flow-bin to version 0.40.0 (#2390) chore: Update `remark-*` modules (#2391)
No description provided.