Support for "let"

[lll000111]

"const" already is at least possible to use, even though it's the var semantics. Using let though results in a SyntaxError. I think let could at least be supported like const already is even now, full support notwithstanding.

[svaarala]

Adding this would be easy, the only question really is whether it will be more confusing to support it but with incorrect scoping (this applies to const already though, as you pointed out).

[martinetd]

Hi,

I've run into a site using these on edbrowse. They'd actually be fine with var semantics here so having that instead of a syntax error.

As you wrote ages ago this is pretty easy, I've monkeyed something that appears to work here if that's of any help (although I'm not sure on how that strings.yaml generates other files?):
martinetd@0701a46

I can open a PR with it after adding some note in the documentation / tests but honestly would prefer the topic of "is a partial feature better than nothing" first; I definitely agree it's confusing, it just so happens that my use case in the wild would work with var instead...

Thanks!

[svaarala]

The diff looks quite good to me at first glance -- feel free to open a PR. I can write some partial feature tests and release documentation as a follow-up if you prefer.

[heikkitx]

Hi Guys,

I agree that allowing let keyword without standard behavior may confuse someone. However it would also allow reusing code that uses 'let' and in my books benefits of reusing code outweighs possible confusion.

If I would have a vote, it would go to const-like let support.

[martinetd]

I had pretty much totally forgotten about it, started looking at how tests work just now. I probably should have taken svaarala's offer up last year, but if I don't find time to whip some basic tests/doc up by monday I'll just open a PR with what I had and leave the rest up to them... Sorry for the delay!

[svaarala]

@martinetd Let me know how you want to proceed :) No worries about the delay, I'm struggling to find time myself right now also.

[Squareys]

@martinetd Would be awesome if you could create that PR! :)

Small note:
It seems to fail for let inside for, e.g.:

let size = 2; // works great
for(let i = 0; i < size; ++i) { // "Syntax Error: parse error"
}           

[Squareys]

@svaarala I tried to take @martinetd's diff and add a test case, but I'm still pretty lost on this:
https://github.com/svaarala/duktape/compare/master...Squareys:let-support?expand=1

The ecmatest seem to be failing on current master. Is that expected currently?

(I was able to fix for(let ...), though.)

[martinetd]

@Squareys thanks - please do take over, I'm a bit swamped... I did have a look at how tests run but didn't have the node.js requirements during the little time I could make in a train and that was it since :(

The help is appreciated :)

[svaarala]

If you are OK with Docker, the easiest approach is to use the docker shell:

$ make docker-images
$ make docker-shell-wd

This runs a shell inside a docker image with all dependencies required for development in place.

The work directory will be a copy and any changes are intentionally lost as you exit the Docker shell. (If you want the working copy to be mounted as is, use make docker-shell-wdmount.)

Then inside the shell just:

$ make ecmatest

This should complete with exit code 0. Note that a bunch of known issues are listed, this is expected:

                          test-bi-array-proto-push: fail; 30 diff lines; known issue: array length above 2^32-1 not supported
[...]
SUMMARY: 1109 pass, 32 fail
$ echo $?
0

The summary is a bit misleading, it should say "32 known".

[e2iplayer]

@Squareys
Could you please share solution for:

(I was able to fix for(let ...), though.)

EDIT:
OK I found it.

-if (comp_ctx->curr_token.t == DUK_TOK_VAR) {
+if (comp_ctx->curr_token.t == DUK_TOK_VAR || comp_ctx->curr_token.t == DUK_TOK_LET) {

[Squareys]

@e2iplayer Great, yeah, I have that above link to the diff. Wasn't able to put more time into this, sorry!

[e2iplayer]

@Squareys Squareys
Unfortunately your link is not working most probably because three dots inside.

[Squareys]

@e2iplayer Uhm, it works fine for me 🤔 Otherwise use https://github.com/Squareys/duktape/tree/let-support

[e2iplayer]

@Squareys
Please take a look how this link looks like:

As you see it is short cut with dots.
I had to find it by myself because of broken link...
Anyway thank you very much.

[MonkeybreadSoftware]

We miss let in the parser.
Could we just define let as an alias vor var keyword, so most cases just work fine?