Skip to content

Conversation

@pd4d10
Copy link
Contributor

@pd4d10pd4d10 commented May 16, 2021

This PR migrates expressions such as a ? a.b : c to a?.b ?? c

Codemod script:
https://github.com/pd4d10/nodejs-codemod/blob/main/src/optional-chaining.ts

Also see:
#38609

This PR migrates expressions such as `a ? a.b : c` to `a?.b ?? c` Codemod script: https://github.com/pd4d10/nodejs-codemod/blob/main/src/optional-chaining.ts
@aduh95
Copy link
Contributor

I'm not sure we want to do that after #38245...
In any case this should be split into several PRs, it's less of a burden to review smaller PRs and we benchmark it more efficiently.


getfd(){
returnthis.#channel ? this.#channel.fd:undefined;
returnthis.#channel?.fd??undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
returnthis.#channel?.fd??undefined;
returnthis.#channel?.fd;

returnfalse;
constrequest=stream[kRequest];
returnrequest ? request.readable:stream.readable;
returnrequest?.readable??stream.readable;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
returnrequest?.readable??stream.readable;
returnstream[kRequest]?.readable??stream.readable;

functiondebugStreamObj(stream,message, ...args){
constsession=stream[kSession];
consttype=session ? session[kType] :undefined;
consttype=session?.kType??undefined;
Copy link
Contributor

@aduh95aduh95May 16, 2021

Choose a reason for hiding this comment

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

Suggested change
consttype=session?.kType??undefined;
consttype=stream[kSession]?.[kType];


functiononError(msg,err,callback){
consttriggerAsyncId=msg.socket ? msg.socket[async_id_symbol]:undefined;
consttriggerAsyncId=msg.socket?.[async_id_symbol]??undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
consttriggerAsyncId=msg.socket?.[async_id_symbol]??undefined;
consttriggerAsyncId=msg.socket?.[async_id_symbol];

@pd4d10
Copy link
ContributorAuthor

I'm not sure we want to do that after #38245...

OK. Given this information, I guess we should hold it until the performance issues solved.

Copy link

@MifrillMifrill left a comment

Choose a reason for hiding this comment

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

Those changes look great, and I see the potential performance issue was also taken into consideration, that's awesome, however would like to raise a hand and ask if the chaining is slower than the ternary operator.

📝 CC @ronag#50337 (comment)

@aduh95aduh95 added the stalled Issues and PRs that are stalled. label May 5, 2024
@github-actions
Copy link
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

@github-actions
Copy link
Contributor

Closing this because it has stalled. Feel free to reopen if this issue/PR is still relevant, or to ping the collaborator who labelled it stalled if you have any questions.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stalledIssues and PRs that are stalled.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pd4d10@aduh95@jasnell@Mifrill