Closed Bug 903755 Opened 7 years ago Closed 6 years ago
Additional builtin objects' methods break Sugar
SugarJS adds serveral functions to Array and other builtins' prototypes. Some of them conflict with ones added in ES6. At least, the following functions are affected: - Array#find - Array#findIndex - Array#repeat - String#repeat Not only might there be more already, it's also somewhat likely that additions in later editions of the standard will conflict. IMO, we should try to convince the SugarJS maintainers to abandon the approach of adding functions with possibly-conflicting names to any builtins' prototypes. They could either focus on polyfilling harmonious builtin additions per spec, or, potentially in addition, add functions with names that are extremely unlikely to collide. Something like `sugarEach` or at least `sEach` would work.
(In reply to Till Schneidereit [:till] from comment #0) > At least, the following functions are affected: > - Array#find > - Array#findIndex > - Array#repeat > - String#repeat Correction: - Array#find - Array#findIndex - Array#from - String#repeat
V8 has also recently implemented the same ES6 methods, so this library might be broken on Chromium too. http://code.google.com/p/v8/issues/detail?id=2776 Array.find http://code.google.com/p/v8/issues/detail?id=2777 Array.findIndex http://code.google.com/p/v8/issues/detail?id=2799 String.repeat
http://sugarjs.com/api/String/repeat won't be a problem, as it works the same as the native function.
No longer blocks: harmony:strrepeat
(In reply to plummer.andrew from comment #5) > SugarJS creator here. > Now to the main part. I have already taken some flak for this on Twitter, > and I think it's with very, very good reason. I have given a good share of that and mean to say that there is nothing personal. I've been tweeting mostly to educate other devs on what to avoid doing. A bunch of people (like myself) keep repeating over and over "don't override built-ins, that threatens the future evolution of the platform". But that's all theoretical, so few really understand and even fewer care. An example of what happens when one doesn't follow the advice is an occasion to re-explain the advice in a more concrete way. Anyway, no bad feelings on my side and importantly nothing personal. > First, I want to make it clear that these collisions aren't taken lightly > 1. Sugar needs to change. There is no other way. In fact, part of the > alignment to upcoming spec was already in progress already. It will be up to > the community to decide *how* it changes, but the fact remains that it has > to. > > 2. Regarding this bug it is my opinion that it is invalid and should be > closed. It isn't the responsibility of those implementing spec to be > concerned with environments that go against it, no matter how small, no > matter intentional or not. Dealing with the repercussions of how new spec > will affect Sugar users should be the responsibility of the Sugar community > alone. I wish I could agree, and I am thankful of your sense of responsibility (it's really not all library maintainers who have this sense of responsibility towards the web ecosystem), but that's not how the web works unfortunately. To make it short (I'll probably write a longer more detailed explanation as a blog post soon), web browsers have an incentive to never break a website. If one browser breaks a website, users will just change of browser and that means decrease in the "guilty" web browser market share. No browser wants to take the risk of decreasing its user base/market share. So, if some websites uses sugarJS and introducing native Array.prototype.find breaks them, web browsers will try to see if the websites can be fixed (this is what tech evangelism bugs are about at Mozilla at least). If there are too many broken websites or if some websites are unmaintained, web browsers will just choose to not introduce Array.prototype.find, regardless of how much library users feel responsible for their mistakes. Browsers (and consequently standards which follow browsers and not the other way around) make decisions based on number and importance of broken websites. That's just how it is regardless of whether we like it or not (I've been aware of that for quite some time and still hate it, but accept it :-) ) Despite your sense of responsibility, this bug will have to remain open until everyone has an understanding of how many websites are at stake and the introduction of new built-ins would break. I guess this bug will be closed if either there is sufficient proof that few enough websites are affected or too many websites are affected. Fixing SugarJS, the library, is a good thing for the long term and future websites, but doesn't help with (worldwide!) deployed copies. One way you can help to improve the short term situation: 1) write patches for every single released version of SugarJS that people might be using in the wild (I'm aware it can be a lot of work and I suggest this idea only out of necessity). The idea is to create drop-in replacements for what people are using and that don't conflict with standard methods. Idea: https://bugzilla.mozilla.org/show_bug.cgi?id=897784#c2 2) (Assuming you have a communication channel to find all (or at least most) people using SugarJS for their websites) : tell people to upgrade to the patched version. 3) Keep people posted, at least through this bug. Thanks!
> To make it short (I'll probably write a longer more detailed explanation as > a blog post soon), web browsers have an incentive to never break a website. > If one browser breaks a website, users will just change of browser and that > means decrease in the "guilty" web browser market share. > No browser wants to take the risk of decreasing its user base/market share. Right, I see your point. I guess that's just the way things work. On the bright side, I think that the impact for this particular ticket should be low. For one, Sugar's traction has been good, but not outstanding, probably for obvious reasons. Additionally, the number and nature of affected methods here is quite small. In fact, even in applications using them, provided calls to find and findIndex have only one argument, the behavior should be identical (don't have the data but I doubt that the second two arguments are commonly used -- also even if passing "startIndex", which would result in it being turned into the function context in a native implementation, still would not likely cause any hard-errors. In most scenarios it would simply result in looping over a few extra elements). String#repeat should be nearly identical. > One way you can help to improve the short term situation...write patches...upgrading.. Ok going to have a look into this right away, and will try to think about other ways to reach people as well.
Andrew, thanks a lot for the stance you're taking here - very much appreciated! We'll probably have to be a bit patient about this, and wait for other browsers to also implement these methods to share the pressure, there's two things you can do that should help quite a bit: - quickly release a new version of SugarJS that changes the problematic methods - give your users clear instructions for updating and urge them to do so ASAP While it's likely that not all of your users are going to receive the message right away, you're in a much better position for reaching them then we are.
New version is being worked on right now and will land very soon. It introduces many changes though so will also do a quick patch for older versions. One they're gtg I'll try to get the word out in whatever way I can!
Ok the new version is out! The latest release: 1. Completely aligns with the ES6 spec. To be clear, that doesn't mean it polyfills all ES6 methods, but the ones that were in conflict with Sugar are now following the spec so they are effectively serving as polyfills (again, they were *mostly* following the spec before, but not 100%). 2. Effectively clobbers any methods it defines that are *not* flagged as polyfills. A tough decision to be sure (updated the main site to explain the rationale behind it), but this will effectively prevent any issues arising from changes in spec going forward, and should prevent any more tickets like these. To be 100% clear though, the goal for Sugar is staying well ahead of actual implementations of spec changes in any case. Additionally I released a patch available here http://bit.ly/19FrJKx for currently existing legacy versions for which this ticket might be a problem. This patch essentially does the same thing as above, but only for the specific colliding methods and only if they already exist. Have already announced these changes through my main channels (blog, Twitter, site), but will also have a look at reaching out directly to larger sites for which this might be an issue.
Andrew, thanks again for actively working on fixing this in SugarJS! Let's hope people quickly adopt the new version, then. :)
Assignee: english-other → nobody
Status: NEW → RESOLVED
Closed: 6 years ago
Component: English Other → Desktop
Resolution: --- → FIXED
Whiteboard: [lib] [js]
Product: Tech Evangelism → Web Compatibility
You need to log in before you can comment on or make changes to this bug.