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

Issues if array has Getter #478

Closed
dadiborn opened this issue May 14, 2020 · 5 comments
Closed

Issues if array has Getter #478

dadiborn opened this issue May 14, 2020 · 5 comments

Comments

@dadiborn
Copy link

dadiborn commented May 14, 2020

Issue description or question

If Array contains Getters/Setters, when you try to expand it inside Value Explorer:

1 - Error in Value Explorer: Cannot delete property '1'...

2 - in 'pseudo' array new value will be added
 for no reason
3 - getting value will be undefined
quokka_bug

Sample code

let size = 2;
const real = [1,2,3,4];
const pseudo = Array(size);
for (let i = 0; i < size; i++) {
    Object.defineProperty(pseudo, i, {
        get(){
            return real[i]
        },
        set(value){
            return real[i] = value;
        }
    });
}

real;//?
pseudo;//?
pseudo[0]//?

Results are same on VSCode and Webstorm.

OS name and version

Windows 10

@smcenlly
Copy link
Member

Thanks for reporting the problem. We're not quite sure exactly what's going on yet, but we can duplicate the issue outside of Quokka using:

let size = 2;
const real = [1,2,3,4];
const pseudo = Array(size);
for (let i = 0; i < size; i++) {
    Object.defineProperty(pseudo, i, {
        get(){
            return real[i]
        },
        set(value){
            return real[i] = value;
        }
    });
}

pseudo.splice(0, pseudo.length);

Node.js returns:

pseudo.splice(0, pseudo.length);
       ^

TypeError: Cannot delete property '1' of [object Array]
    at Array.splice (<anonymous>)
    at Object.<anonymous> (C:\Temp\facebook-jest\examples\mongodb\index.js:15:8)
    at Module._compile (internal/modules/cjs/loader.js:1158:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1178:10)
    at Module.load (internal/modules/cjs/loader.js:1002:32)
    at Function.Module._load (internal/modules/cjs/loader.js:901:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:74:12)
    at internal/main/run_main_module.js:18:47

Interestingly, if we set the array before it's defined, the code runs successfully:

  let size = 2;
  const real = [1,2,3,4];
  const pseudo = Array(size);
+ pseudo[0] = undefined;
+ pseudo[1] = undefined;
   for (let i = 0; i < size; i++) {
       Object.defineProperty(pseudo, i, {
           get(){
               return real[i]
           },
           set(value){
               return real[i] = value;
           }
       });
   }

   pseudo.splice(0, pseudo.length);

We're going to investigate further as to why this is occurring but it may be that defining the property on the array as you are doing is not supported by node (at least not without breaking some functions of the Array).

@dadiborn
Copy link
Author

Then I will say Thank You Too twice!
First: Need to find another approach to get value :)
I have flat array and wanted to get part of it as array where changes will affect original values in flat array. (If you have any idea that would be off topic but super appreciated)
Second: Thank you for that super fantastic products with astral and hard to type titles (Wallaby...Quokka) - Absolute winner in most useful extensions ever made!

@smcenlly
Copy link
Member

We've found that this is caused by Configurable attribute specifying that the property cannot be deleted from the object (mentioned in defineProperty docs), which is why slice() is failing. By default, the configurable property is false, which is what is causing Wallaby and Array.slice() to fail.

As a short term workaround, you may set configurable to true to get around the issue. Unless you very specifically don't want the array object itself being modified, this should let you do what you want to do. We modify the value as we report it in value explorer to prevent against large Arrays and other optimizations that are important to us.

We're continuing to investigate a proper fix for this that will allow you to use configurable true and still have Wallaby correctly display your values.

@dadiborn
Copy link
Author

Thank you! Yes I just found that while smoking and came to response about configurable default to false in defineProperty but you was faster :) , Will try it now. Thanks for so detailed explanation!

@smcenlly smcenlly reopened this May 15, 2020
@smcenlly
Copy link
Member

This has been fixed and is available in the latest version of Wallaby core, v1.0.893.

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

No branches or pull requests

2 participants