-
-
Notifications
You must be signed in to change notification settings - Fork 17
Require Node.js 8, add TypeScript definition #16
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
Require Node.js 8, add TypeScript definition #16
Conversation
| #### options | ||
|
|
||
| Type: `Object`<br> | ||
| Type: `object`<br> |
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.
Not sure about changing this. Technically, Object is correct. It's only TS that has a special object type. In normal sense, primitives are lowercase and non-primitives pascal-case. Thoughts? I haven't completely made up my mind with this or going with exact TS types.
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.
Not sure either. I saw that you refactored quite a few types to TS notation and thought that you've started using TS types in your docs. So this would be TypeScript's "primitive" object type that accepts any object. The Object type is the object constructor and should not be used for plain objects at least in TS.
I don't have a strong preference here, I just wanted to be consistent with TS.
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.
Good point. Let's go with object. (See: sindresorhus/meta#1)
index.js
Outdated
| if (typeof str !== 'string') { | ||
| throw new TypeError(`Expected \`input\` to be a \`string\`, got \`${typeof str}\``); | ||
| module.exports = (string, count = 1, options = {}) => { | ||
| const options = {indent: ' ', includeEmptyLines: false, ...options}; |
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.
| const options = {indent: ' ', includeEmptyLines: false, ...options}; | |
| options = { | |
| indent: ' ', | |
| includeEmptyLines: false, | |
| ...options | |
| }; |
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.
Just out of curiosity, why don't you use Prettier? It seems to produce quite similar code style that you already use and is overall a tool that doensn't need much configuration very much in the style of your own tools.
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.
I makes too many wrong choices, in my opinion, at the level of being annoying for me. I'm very opinionated about code-style...
The above is one example. I prefer having object and array items on separate lines, always. Makes it easier to read and edit.
No description provided.