Use of getters/setters removes any chance of compatibility with IE #2

Closed
kirbysayshi opened this Issue Apr 21, 2011 · 13 comments

Projects

None yet

4 participants

@kirbysayshi

First off, great job on this library. Next time I need a scene graph, I may put this thing through the ringer with some game dev.

I was evaluating using oCanvas in a project for work, where, unfortunately, the requirements are >= IE6. I hoped that with a canvas shim everything would be ok (I'm using FlashCanvas)...

GAH! Getters and setters! Foiled!

I know it's a long shot that you'd even consider removing them, but I figured I'd at least report this in the event that others are wondering... and to save them some time.

I realize it's non-trivial and would break the current API. Also, I suppose it's sort of trivial in difficulty to remove getters/setters and replace them with function calls, but would still take time (which I don't believe I have enough for this work project).

Let's say I got really motivated, and replaced all the getters/setters... what would you think of that?

@erbil
erbil commented Sep 15, 2011

I second this request, other than getters and setters most of the code would work with ExplorerCanvas

@koggdal koggdal added a commit that referenced this issue Sep 28, 2011
@koggdal Switched accessor style to support IE9 and above. Fixes parts of issue
…#2.

Added Xccessors as a polyfill for Object.defineProperty() and
Object.getOwnPropertyDescriptor() and switched to using these methods
when defining and looking up accessors. This increases browser support
and IE9 and above are now supported.
6367230
@koggdal koggdal was assigned Sep 28, 2011
@koggdal
Owner
koggdal commented Sep 28, 2011

I'm sorry for not responding earlier, but I wanted to get more into how this all works in different browsers. When I started developing oCanvas, no released IE version supported canvas (IE9 wasn't out then), so IE was something I didn't think about at all. Since all other browsers at that time seemed to support accessors, I decided to use that to enable a really good API (changing properties can sometimes be much more intuitive than calling functions).

What I should have thought of more though was how to implement it. I didn't go very deep in my research, and I ended up using the legacy methods for this (defineGetter etc). When IE9 was released, they didn't support that style, but they do support the ES5 style. IE9 was released just a few days before I released oCanvas, so there was not really any time to change this then.

Since the initial release I've been really busy and haven't worked that much at all with oCanvas. In the recent days now I've taken up development again and uploaded some fixes I had locally, fixed some issues, and looked into the IE problem.

I have now included Xccessors to polyfill Object.defineProperty() and Object.getOwnPropertyDescriptor() in non-supported browsers, and switched to using the ES5 style. This means oCanvas will now be supported in IE9 and above as well!

As good as this is, I realize some people will want support for IE8 and below. Personally, I feel IE6-7 are dead to me. It is no priority for me to support that old browsers. IE8 is still not totally dead to me, but in this case I think it has to be.

IE8 doesn't support canvas, so we then need to use some library like ExplorerCanvas or FlashCanvas. From what I've seen, ExplorerCanvas does not perform well when there are animations involved. That rules out a lot of the things you can do with oCanvas, and I'm not going to change things to support a browser that you need to polyfill completely, and then get only a small portion of the API to work well. FlashCanvas seems to perform better, but the support for the canvas API varies between versions of FlashCanvas, and there are still some quirks.

Even if there would be a perfect polyfill library for canvas, IE8 has more problems — it only supports accessors on DOM objects. I had a thought about a special solution for IE8 where objects are extending a DOM object to enable accessors, but that would mean a major rewrite of all the code.

I don't think it's worth it to do all this, to get a poor result with a polyfill in a browser that is going to be dead in a while anyway. So I'm not going to support IE8 and below. If you really need support in older browsers, maybe you should create your app in a more traditional manner using Flash or something else that is supported.

@koggdal koggdal closed this Sep 28, 2011
@jslegers

Excanvas performance in IE6 is quite poor, but in IE8 it's more than enough for common use cases.

The Flot charting plugin used to be one of the more popular jQuery libraries a couple of years ago. Dating back to 2007, it adopted Excanvas before there was any mention of native canvas support in IE. In IE8, it actually runs relatively smootly, even when the Canvas is animated --> http://www.flotcharts.org/ .

With regards to the accessors issue, I'm wondering if https://github.com/eligrey/Xccessors might help?

@koggdal
Owner
koggdal commented Feb 19, 2013

Can you show me a demo with excanvas, using animations with more than a few objects, using gradients? oCanvas is not just for simple charts, so comparing with flotcharts is a bit unfair.

Yes, Xccessors is included in oCanvas, which helps with supporting the different syntaxes for accessors. It does not help with the fact that IE8 only supports it for DOM objects. So even if the performance would be okay with excanvas, I think it would perform worse when you change it so that all objects extend a DOM object.

@jslegers

Fabric.js is a good example of a popular generic Canvas library with Excanvas support. Unfortunately, its Excanvas support seems to have broken with the current release. I posted an issue on their Github as well.

What about extending the window? How does that compare when it comes to performance? IE8 does supports getters and setters on the window object.

What do you need accessors for anyway? What exactly is it accessors offer that eg. the design pattern below doesn't offer? It allows me to create custom objects, and add custom properties as well as custom functions on the fly while exposing only a part of the API to other objects. I can't really compare performance as I never used accessors in Javascript, I've been using the code pattern below in my own code without any issues in even IE6 and IE7.

var myClass = (function() {
. . . var some_private_variable;
. . . myClass = {
. . . . . . version : '1.0'
. . . };

. . . some_private_function = function(){
. . . . . . //
. . . }

. . . another_private_function = function(){
. . . . . . //
. . . }

. . . myClass.get = function(){
. . . . . . some_private_function();
. . . . . . return some_private_variable;
. . . }

. . . myClass.set = function(value){
. . . . . . another_private_function();
. . . . . . some_private_variable = value;
. . . }

. . . return myClass ;
})

@jslegers

As I was typing my previous post, Juriy Zaytsev had read the issue I'd posted and fixed the issue.

You could check out http://fabricjs.com/animation-easing/ to see how Fabric.js deals with Excanvas support and animations.

@koggdal
Owner
koggdal commented Feb 19, 2013

You can do a benchmark with extending window and DOM objects if you want to see the performance.
I use accessors to do special things when you get or set a property.
obj.x = 50; does more than just setting the x property, to support positions for child objects in an easy way.

@jslegers

The design pattern I posted allows you to do the same.

In the example, "myClass.get" gets the value of "some_private_variable" but also calls method "some_private_function".

Similarly, "myClass.set" sets the value of "some_private_variable" and also calls method "another_private_function".

"some_private_variable" stays hidden outside the object.

@koggdal
Owner
koggdal commented Feb 19, 2013

This isn't really about public/private state, but more about doing things when a property changes. I want an API where you can do:
obj.x = 50; // 50
obj.x += 50; // 100
With that exact API. With your example, I would need to call a function, which I don't want.

@jslegers

For your specific use case, I prefer semantics like "obj.x.get", "obj.x.add", "obj.x.substract", "obj.x.init" and "obj.x.set". I don't think the use of the accessor syntax you're using is a good idea at all for two main reasons :

  • That coding style is incompatible with older browsers.
  • It reduces the readability of the code. When I see something like "obj.x = 50", I expect only a value of 50 to be assigned to x. I don't expect something to happen in background. Neither do I wish to check with every assignment of this kind if something does happen in background. With a syntax like "obj.x.get" I'm a lot more inclined to look for stuff happening in the background as that's usually the reason this type of syntax is used.

Yet, in the end coding styles have always been and will always be a matter of personal taste.

@koggdal
Owner
koggdal commented Feb 20, 2013

What happens in the background is stuff that the user of the library shouldn't really care about. That's internals, and the purpose of the library, to do stuff for you so creating things with the library is easy. So, as a user, when you set obj.x = 50, you expect the object to move to that position, and also that its children move accordingly. If you have different opinions, you are completely free to create your own canvas library :)

@jslegers

It's not the user of the library I'm worried about. It's potential future contributors.

Maybe I'm oldfashioned, but I still believe the crowdsourcing potential is one of the things that make open source great. If you're the only one who can read your code, that potential goes entirely out the window.

The more readable your code, the more easily people understand what's going on under the hood and the more people are capable of helping you improve your project.

@koggdal
Owner
koggdal commented Feb 20, 2013

Oh, okay. If you're talking about the internal code of oCanvas, it doesn't use accessors that much itself—it's mostly for the public API. Also, they are part of the language, so people should get used to them. Things evolve, and so does the language, which means we as developers will always need to learn new things. I realize not everyone knows accessors, but that's not a reason to change the public API of the library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment