Skip to content
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

FEAT: Implement ?? #1591

Merged

Conversation

0xe
Copy link
Contributor

@0xe 0xe commented Aug 29, 2024

Closes #938

@0xe 0xe force-pushed the scratch/satish/implement-nullish-coalscing-op branch 2 times, most recently from b6365bc to a8d4621 Compare August 29, 2024 13:05
@0xe 0xe marked this pull request as ready for review August 29, 2024 13:15
@0xe 0xe requested a review from andreabergia August 29, 2024 13:16
@rbri
Copy link
Collaborator

rbri commented Aug 29, 2024

@0xe maybe https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_OR_assignment and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_AND_assignment are doable in a similar way.... ;-)

@p-bakker
Copy link
Collaborator

You'll want to uncomment the language/expressions/coalesce section in test262.properties, so all the tests related to this feature will run

@p-bakker
Copy link
Collaborator

@0xe maybe https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_OR_assignment and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Bitwise_AND_assignment are doable in a similar way.... ;-)

While at it, why not toss in the optional chaining operator 🤪 See #936 and #937

@nabacg
Copy link
Contributor

nabacg commented Aug 29, 2024

still WIP #1593 but I couldn't help myself to give it a try!

@p-bakker
Copy link
Collaborator

Pipeline fails cause not all tests from language/expressions/coalesce pass

@p-bakker
Copy link
Collaborator

@rbri I assume you meant ||= and &&= instead of |= and &=, as I think the latter two bitwise operators (which you linked to) are already supported

@andreabergia
Copy link
Contributor

@0xe 0xe force-pushed the scratch/satish/implement-nullish-coalscing-op branch 2 times, most recently from d90e7a3 to 29f19c1 Compare August 30, 2024 19:51
@0xe 0xe requested a review from rbri August 30, 2024 19:52
@0xe
Copy link
Contributor Author

0xe commented Aug 30, 2024

Pipeline fails cause not all tests from language/expressions/coalesce pass

Thanks, I've fixed the failing test.

@gbrail
Copy link
Collaborator

gbrail commented Sep 8, 2024

This looks good to me -- any thoughts from anyone else?

One minor question -- the addition of a "NULLISH_COALESCING" token is a name we'll have to live with for a long time. Are we sure that we like that name -- does it match how people talk about this feature elsewhere in the JavaScript world? Or is another name more appropriate.

+1 for adding a French translation, BTW!

@gbrail
Copy link
Collaborator

gbrail commented Sep 8, 2024

Also, if others are OK then please rebase it so that we can merge it. Thanks!

@0xe
Copy link
Contributor Author

0xe commented Sep 10, 2024

This looks good to me -- any thoughts from anyone else?

One minor question -- the addition of a "NULLISH_COALESCING" token is a name we'll have to live with for a long time. Are we sure that we like that name -- does it match how people talk about this feature elsewhere in the JavaScript world? Or is another name more appropriate.

+1 for adding a French translation, BTW!

Thanks. We picked NULLISH_COALESCING based on what MDN calls it. Happy to change if another name is more commonly used in the community.

@0xe 0xe force-pushed the scratch/satish/implement-nullish-coalscing-op branch from 07123b3 to 304e178 Compare September 10, 2024 02:16
@0xe
Copy link
Contributor Author

0xe commented Sep 10, 2024

Also, if others are OK then please rebase it so that we can merge it. Thanks!

I've rebased it against master.

@gbrail gbrail merged commit 5c8707f into mozilla:master Sep 11, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ES2020 Nullish Coalescing Operator
6 participants