microsoft
Refactor `getTypeOfSymbol` for Improved Readability and Maintainability
Acknowledgement
- [x] I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.
Comment
Hello:)
I have noticed that the getTypeOfSymbol function in the compiler is overly complicated and could be simplified.
Your suggestion is creating an unnecessary object and arrays every time the function is called. And IMO is more complicated and much less readable.
Here's a declined PR that has the exact same change on another location: #57375
Relevant comments:
This incurs an object allocation on every call, and the object gets bigger every time we add more stuff.
and
There are over 2,000 issues in our Backlog that we're accepting PRs for. Please don't send stylistic/formatting/"cleanup" PRs that don't address an identified issue. Of all the things we could use help with, reformatting our code isn't one of them 😅
Here's a declined PR that has the exact same change on another location: #57375
Relevant comments:
This incurs an object allocation on every call, and the object gets bigger every time we add more stuff.
and
There are over 2,000 issues in our Backlog that we're accepting PRs for. Please don't send stylistic/formatting/"cleanup" PRs that don't address an identified issue. Of all the things we could use help with, reformatting our code isn't one of them 😅
Why not add it to the Pre-Declined Feature Requests section in the FAQ if that is the case? I have re-read the FAQ multiple times to make sure my pr is allowed and it is.
🤷♂ In the end it's up to the TypeScript team (which I am not a member of).
The PR template does mention:
There is an associated issue in the
Backlogmilestone (required)
And in the mentioned "Contributing" file it talks about contributing bugs and contributing features - yours is neither or a bug nor a feature.
Also, readability and maintability is very subjective. I find your changes neither more readable nor more maintainable. But definitely having worse performance due to the additional allocations.
Why not add it to the Pre-Declined Feature Requests section in the FAQ if that is the case?
This is not a feature request.
Why not add it to the Pre-Declined Feature Requests section in the FAQ if that is the case?
People who aren't reading CONTRIBUTING.md are likely not reading the FAQ either; I don't see how that would help.
Why not add it to the Pre-Declined Feature Requests section in the FAQ if that is the case?
People who aren't reading CONTRIBUTING.md are likely not reading the FAQ either; I don't see how that would help.
I am very sorry