-
Notifications
You must be signed in to change notification settings - Fork 858
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: Support default parameters #1640
Conversation
Thanks -- this has been outstanding for a long time, and it makes a lot more things work so I'm going to merge it. I notice that there are test262 tests that actually pass but are marked as failing, so I'm going to fix that and manually merge. Thanks for working on this! |
Merged manually with some test262 fixes -- looking forward to more! |
Thank you! |
@0xe would you say we can close #756 now as well? Unless you'll open a PR for the three things you mentioned (that aren't supported yet) very shortly, cloud you maybe create a follow-up issue, so we don't lose track of it? Just checking: this PR didn't move the needle on rest/spread syntax anywhere, correct? |
Yeah, I will create issues for the TODO items from this PR tonight. There should be no changes to rest/spread from this PR. For #756, I think the basic things should work. Yeah, I do think it makes sense to close that and perhaps create issues for things which are still broken. |
Any chance you can have a look what, if anything, is still not working wrt default values in destructuring assignments? I would think support/issues between destructuring assignments and default function parameter values to be quite similar, but I might be wrong. For now I'm holding of on closing #756 a bit until we have some (more) insight into this |
@0xe Wow amazing, 1k lines changed. Thanks for your work! I remember opening the issue 4 years ago as a complete Java noob. Time flies. |
Added an issue for known unimplemented things from this PR: #1641 |
Makes sense. I will take a look at what's pending with destructuring assignments and update #756. From my quick check in the shell, following expressions evaluate correctly: js> let [a, b] = [1, 2];
js> a
1
js> b
2
js> let [c=3, d=4] = [];
js> c
3
js> d
4
js> let {e = 5, f = 6} = {};
js> e
5
js> f
6
js> let {g = 7, h = 8} = {g: 9, h: 10}
js> g
9
js> h
10 One thing that should be addressed in #1641 is reading |
Adds support for default values in function parameters and default values in destructuring assignment.
What's not implemented yet, that I plan to implement subsequently:
(Added ignored tests for these in DefaultParametersTest)
(same changes as #1481, but squashed and on top of master)