-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[typescript-rxjs] refactor to arrow functions and short hand object creation #3077
[typescript-rxjs] refactor to arrow functions and short hand object creation #3077
Conversation
…nds where possible
…per function in api controllers
…ery and header params
…redError and querystring
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
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.
LGTM
However, I am not in favor of replacing methods with function properties, as this prevents the class methods from being overridden/extended (see https://stackoverflow.com/questions/39156315/function-property-vs-method/43044290). You can e.g. try https://www.typescriptlang.org/play/index.html#src=class%20A%20%7B%0D%0A%20%20%20%20a%20%3D%20()%20%3D%3E%20alert('A.a')%3B%0D%0A%20%20%20%20b()%20%7B%0D%0A%20%20%20%20%20%20%20%20alert('A.b')%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0Aclass%20B%20extends%20A%20%7B%0D%0A%20%20%20%20a()%20%7B%0D%0A%20%20%20%20%20%20%20%20super.a()%3B%0D%0A%20%20%20%20%20%20%20%20super.b()%3B%0D%0A%20%20%20%20%7D%0D%0A%7D
}) | ||
.join('&'); | ||
|
||
// alias fallback for not being a breaking change |
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.
nice
…n header or query params
I missed the condition to add properties to the |
PR checklist
./bin/
to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh
,./bin/openapi3/{LANG}-petstore.sh
if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\
. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.master
,4.1.x
,5.0.x
. Default:master
.Description of the PR
throwIfRequired
RequiredError
(this is a breaking change if people are using it outside the generated code)headers
andquery
objects to be created inline to be less verbose and skip redundant null checksThis all results in a noticeable code savings.
In my current react project with CRA2 (9 controllers and 14 endpoints) I get a build with 4368 characters less which is about 4%, although file size after gzip it is only 230 Bytes which is about 1%.
@TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10)