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

Object is not extensible when using minified version #12

Closed
smiffy6969 opened this issue Apr 29, 2016 · 11 comments
Closed

Object is not extensible when using minified version #12

smiffy6969 opened this issue Apr 29, 2016 · 11 comments

Comments

@smiffy6969
Copy link

When using minified version, I get a an error 'Object is not extensible' when trying to push or splice to an array.... if I Array.isArray() from console, the result is false..... strange.....

If I switch back to non minified, the issue goes away.

Anyone else noticed this?

@samthor
Copy link
Contributor

samthor commented May 1, 2016

If you proxy an array, it will always be made immutable (i.e., sealed).

When your code is in strict mode, you'll get a TypeError when you try to modify that array. If you're not in strict mode, it'll fail silently (not good). Is your calling code in strict mode?

@smiffy6969
Copy link
Author

smiffy6969 commented May 1, 2016

No I am proxying an object with various properties, one property is an array. When I try to push or splice a value to the property, I get this error on the minified version.

I am calling in strict mode, it is ES6 compiled code. ES6 is always strict.

Regardless of this, working in non minified and not working in minified kinda says..... These two things are not working the same, do they not?

Paul

samthor added a commit that referenced this issue May 2, 2016
@samthor
Copy link
Contributor

samthor commented May 2, 2016

I had a go at trying to repro this here. I then loaded the tests in Safari (uses minified, due to lack of ES6 support) and Chrome (non-minified) and they both pass.

Can you attach concise repro code or modify what I've written there?

@smiffy6969
Copy link
Author

smiffy6969 commented May 2, 2016

export default class Observer {
    static object(obj, fn, deep, prefix) {
        if (typeof this.object === 'undefined') this.object = obj;
        prefix = typeof prefix === 'undefined' ? '' : prefix;
        return new Proxy(obj, {
            set: function(target, prop, value) {
                let old = target[prop];
                target[prop] = value;
                fn(prefix + prop, old, value);
                return true;
            },
            get: function(target, prop) {
                let val = target[prop];
                if (!!deep && val instanceof Object && typeof prop === 'string') return Observer.object(val, fn, deep, prefix + prop + '.');
                return val;
            }
        });
    }
}

The above is used to loop through object data to create a tree of observable objects, forcing a function to be run on change to any bit of the data.

This works fine with non minified version, but fails with minified version (sealed array elements on splice, not extensible on push). Like i say works like a dream on non minified.

While I am here, the more worrying thing here is that I am using chrome, I just completely removed the polyfill, and it works natively too with no problem. I just added the minified version back in, and it's not working...... surely, isn't the polyfill suppose to step back when there is native support, thats the whole point of a polyfill isn't it? I have no way of checking if the non minified version of this code is stepping back or not, but it clearly looks like the minifed version is being used regardless of browser having native or not.

So first, the nested observing on minified is failing, (my dirty tests look like not all changes are being picked up on minifed) but I do not know this library enough yet to comment.

Next, why is the minifed version failing anyway in chrome Version 50.0.2661.86 (64-bit)? surely native support stops the polyfill being loaded, replacing native support?

thanks

@samthor
Copy link
Contributor

samthor commented May 2, 2016

Ok, this was actually a bug: Closure compiler was clobbering the initial scope.Proxy check. I've fixed this now and released a new version. Thanks for the report!

Having said that, your code is proxying each Array object too. And where there's no native support, this will make the array immutable- the polyfill can't treat these specially. For broad browser support, your best bet is to replace the whole array every time you want to change it.

@smiffy6969
Copy link
Author

Ok,

So after a lot of playing around, I finally see what is happening...

This bug with Proxy always being overriden when minified was actually also the reason why it was 'appearing to work' for me when using the non minified (was actually falling back to native, which allows nested Proxies) but the minified version was hard overriding the native proxy, meaning the nested was failing. Makes sense now, ignore the pull request.

I see this issue is now resolved as per things working differently between non minified and minified.

Lastly then, before i go (thanks for being patient by the way)... Is there no way to get nested proxies working ala native, or is this just not possible? I understand 'just replace the whole array' as a response, just wanting to know if I am going to be busting a nut here trying to get this to work a little more closely to native.

thanks

Paul

@smiffy6969
Copy link
Author

So this is a complex issue, it requires engine support to enable mutable arrays..... For now I will see if I can get by some other way, or maybe a dirty checking solution to fall back on for other browsers. This is closed thanks.

@samthor
Copy link
Contributor

samthor commented May 3, 2016

I'm sorry the polyfill hasn't worked for you. It's really a limited set of operations and I'll try to make that clearer in the README. Dirty checking will work, yes, but it's slow as you're no doubt aware :(

@smiffy6969
Copy link
Author

smiffy6969 commented May 4, 2016

thats fine. I should have read the readme a bit better. typical man though, instructions later.... On the plus side I have dropped back to an object.observe polyfill and pretty much have what I need, OO polyfill for when Proxy not present. Just need to tweak it a bit. I have no issue using OO polyfill, as soon as proxy is available it will stop using it. I am only really interested in N-1 anyway. thanks for you help.

One last thing, whilst I have proxy polyfill in the wild still, I did a quick check in my dinner, I was having issues with IE (11 I think, cant remember was on my VM) It was failing parse on the backtick template blocks. So you may want to double check your compatibility, not sure what you are supporting but I had problems with IE and it is normally up to date, didnt try edge at the time)... It was also failing IOS mobile on daughters Iphone 6 (safari and chrome) Not sure if this was the backtick issue too but the end result was the same that i had with IE. So I was not getting good coverage with the polyfill, didn't have time to get proper tests though... ;)

regards

Paul

@alansouzati
Copy link

Hey I'm facing this problem too. I'm trying to use this Polyfill mainly for IE11 that does not support Proxy. and the bundled dependency has the backtick string. IE11 does not support that. So any change that you can use something to transpile that? Or do not use backtick? I would be happy to send a contribution if you guys believe this makes sense.

@samthor
Copy link
Contributor

samthor commented Oct 25, 2017 via email

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

3 participants