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

Filled array displayed like [] in debug repl #6444

Closed
evegreen opened this issue Apr 28, 2016 · 6 comments
Closed

Filled array displayed like [] in debug repl #6444

evegreen opened this issue Apr 28, 2016 · 6 comments
Labels
v8 engine Issues and PRs related to the V8 dependency.

Comments

@evegreen
Copy link

evegreen commented Apr 28, 2016

Steps:

  • create file with this content:
debugger;
  • in console: node debug fileName.js (enter)
  • when stops at breakpoint, enable repl
  • type this: myVar = [{lol: 'lal'}]; (enter)
  • type: myVar (enter)

Expected:
[ { lol: 'lal' } ]

Actual:
[]

PS:

  • if i start simple repl (not debug), this bug don't reproduce
  • i can't reproduce this bug on v5.3.0 on windows 10
  • sorry for my "bad" english
@addaleax
Copy link
Member

Bisecting says this was introduced somewhere in 069e02a, 89f2343, 079973b from #4722.

/cc @nodejs/v8

@addaleax addaleax added the v8 engine Issues and PRs related to the V8 dependency. label Apr 28, 2016
@cjihrig
Copy link
Contributor

cjihrig commented Apr 28, 2016

It looks like something subtly changed from a number to a string. cjihrig@b64463f appears to fix the problem, but I'm unsure if it is the "correct" fix or not.

@addaleax
Copy link
Member

@cjihrig This may be a dumb question, but, like… is there any reason not to just do what the comment says? i.e. check for prop.name === 'length'? Good catch anyway!

That code has been around since 0fa3f2f and I think changing it to that would be correct here anyway.

// debug repl, node v5.11
> a = [1]; a.b = 2; a
[ 1 ]
// normal repl
> a = [1]; a.b = 2; a
[ 1, b: 2 ]

I’d expect these to match.

@cjihrig
Copy link
Contributor

cjihrig commented Apr 28, 2016

Good question. It certainly seems like that would be fine, but there could be some historical context that I don't know about. I can put together a PR and see what people say :-)

@addaleax
Copy link
Member

@cjihrig … if you don’t I will. 😄

@evegreen
Copy link
Author

Thank you very much! =)

cjihrig added a commit to cjihrig/node that referenced this issue May 2, 2016
This commit allows all array properties to be printed except for
"length". Previously, this filter was applied by checking the
type of each property. However, something changed in V8, and
array elements started coming through as numeric strings, which
stopped them from being displayed.

Fixes: nodejs#6444
PR-URL: nodejs#6448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Fishrock123 pushed a commit that referenced this issue May 4, 2016
This commit allows all array properties to be printed except for
"length". Previously, this filter was applied by checking the
type of each property. However, something changed in V8, and
array elements started coming through as numeric strings, which
stopped them from being displayed.

Fixes: #6444
PR-URL: #6448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
joelostrowski pushed a commit to joelostrowski/node that referenced this issue May 4, 2016
This commit allows all array properties to be printed except for
"length". Previously, this filter was applied by checking the
type of each property. However, something changed in V8, and
array elements started coming through as numeric strings, which
stopped them from being displayed.

Fixes: nodejs#6444
PR-URL: nodejs#6448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 2, 2016
This commit allows all array properties to be printed except for
"length". Previously, this filter was applied by checking the
type of each property. However, something changed in V8, and
array elements started coming through as numeric strings, which
stopped them from being displayed.

Fixes: #6444
PR-URL: #6448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
This commit allows all array properties to be printed except for
"length". Previously, this filter was applied by checking the
type of each property. However, something changed in V8, and
array elements started coming through as numeric strings, which
stopped them from being displayed.

Fixes: #6444
PR-URL: #6448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
MylesBorins pushed a commit that referenced this issue Jun 24, 2016
This commit allows all array properties to be printed except for
"length". Previously, this filter was applied by checking the
type of each property. However, something changed in V8, and
array elements started coming through as numeric strings, which
stopped them from being displayed.

Fixes: #6444
PR-URL: #6448
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests

3 participants