Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Conversation

@a-b-r-o-w-n
Copy link
Contributor

@a-b-r-o-w-n a-b-r-o-w-n commented May 19, 2020

@cwhitten cwhitten changed the title build: update typscript, eslint, prettier and more build: update typescript, eslint, prettier and more May 19, 2020
@beyackle
Copy link
Contributor

This is a huge set of file diffs to look through - am I correct that the biggest changes here are to put parentheses around function arguments (e.g. (x) => x+2) and to alphabetize props in components everywhere? I support the former, but I don't know if I agree with the latter. It does make props easier to find in a long list, but it means they can't be grouped functionally - e.g. if I wanted to put all of the onEvent props at the end of a component as a convention.

@a-b-r-o-w-n
Copy link
Contributor Author

a-b-r-o-w-n commented May 19, 2020

@beyackle You are correct. You are also in luck, the alphabetized props already groups event handlers on* together at the end!

edit: the parens for arrow functions is now a prettier default. The reasoning is that it makes it easier to 1) add additional parameters and 2) add param typing.

Also, the vast majority of changes to src files were eslint --fix changes. There were just a few things that the new ts compiler started complaining about, and I tried to isolate those changes in their own commits.

@a-b-r-o-w-n
Copy link
Contributor Author

@beyackle I'm open to discuss new / changes to eslint rules. With respect to sorting props, I like that you don't have to think about it -- every prop has its place!

@beyackle
Copy link
Contributor

Giving it some more thought, I think I agree with the alphabetized props. It's one fewer way code can be changed without changing its meaning. If we decide it's too annoying, we can revert that part.

Copy link
Contributor

@beyackle beyackle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The automated changes look pretty good, but there are a few small things I think still need to be changed.

Copy link
Contributor

@beyackle beyackle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 🚢

@a-b-r-o-w-n a-b-r-o-w-n merged commit d104c61 into master May 21, 2020
@a-b-r-o-w-n a-b-r-o-w-n deleted the abrown/build/update-typescript branch May 21, 2020 15:45
lei9444 pushed a commit to lei9444/BotFramework-Composer-1 that referenced this pull request Jun 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[chore] update typescript, eslint, prettier

3 participants