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

Document jQuery.easing with one argument #912

Closed
markelog opened this Issue Apr 18, 2016 · 12 comments

Comments

Projects
None yet
6 participants
@markelog
Member

markelog commented Apr 18, 2016

We tried to remove additional arguments it in 3.0, turns out there is a lot of plugins using them, so we decided to deprecate it first before removing in some next major, but we can't deprecate something internal, so we decided to just document it as one argument method.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Apr 18, 2016

Member

In particular the jQuery Easing plugin which is in use by about 8 percent of the top million sites. The impact is probably more when you realize the code is small and may have been copy/pastaed to other places as well.

Member

dmethvin commented Apr 18, 2016

In particular the jQuery Easing plugin which is in use by about 8 percent of the top million sites. The impact is probably more when you realize the code is small and may have been copy/pastaed to other places as well.

@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Apr 18, 2016

Member

It's entirely possible that those stats are completely wrong too. You'd have to know how BuiltWith is detecting the easing plugin and whether that logic can differentiate between jQuery Easing and jQuery UI.

Member

scottgonzalez commented Apr 18, 2016

It's entirely possible that those stats are completely wrong too. You'd have to know how BuiltWith is detecting the easing plugin and whether that logic can differentiate between jQuery Easing and jQuery UI.

@kswedberg

This comment has been minimized.

Show comment
Hide comment
@kswedberg

kswedberg Apr 18, 2016

Member

Presumably, though, if BuiltWith were including jQuery UI in its results for jQuery Easing, it would claim more than 18 percent, which is what BuiltWith says jQuery UI has out of the top 1 million. Even if only half of jQuery UI installs included UI effects core, that would still put easing over 9%. So, I'm guessing they have some way of differentiating the two.

Member

kswedberg commented Apr 18, 2016

Presumably, though, if BuiltWith were including jQuery UI in its results for jQuery Easing, it would claim more than 18 percent, which is what BuiltWith says jQuery UI has out of the top 1 million. Even if only half of jQuery UI installs included UI effects core, that would still put easing over 9%. So, I'm guessing they have some way of differentiating the two.

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Apr 18, 2016

Member

In any case it seems like a good idea to err on the side of caution and not change the code in 3.0. The ticket here is just to document that the easing functions get one argument.

Member

dmethvin commented Apr 18, 2016

In any case it seems like a good idea to err on the side of caution and not change the code in 3.0. The ticket here is just to document that the easing functions get one argument.

@scottgonzalez

This comment has been minimized.

Show comment
Hide comment
@scottgonzalez

scottgonzalez Apr 18, 2016

Member

Even if only half of jQuery UI installs included UI effects core, that would still put easing over 9%.

Detection and overlaps are somewhat complicated. The two main methods are detecting URL patterns in <script> and detecting code patterns in script content. A large portion of jQuery UI users will just grab the entire suite from a CDN. In this case, BuiltWith probably doesn't parse the code since they already know that it points to jQuery UI. So even though many jQuery UI users will have the easing code, it wouldn't count as a hit for jQuery Easing. On the other hand, anyone who does a custom jQuery UI build and concatenates code, renames files, etc. would require code parsing. In this case, inclusion of jQuery UI could result in a hit for jQuery Easing in addition to a hit for jQuery UI.

Anyway, it sounds like it doesn't matter. So I guess this issue should be closed.

Member

scottgonzalez commented Apr 18, 2016

Even if only half of jQuery UI installs included UI effects core, that would still put easing over 9%.

Detection and overlaps are somewhat complicated. The two main methods are detecting URL patterns in <script> and detecting code patterns in script content. A large portion of jQuery UI users will just grab the entire suite from a CDN. In this case, BuiltWith probably doesn't parse the code since they already know that it points to jQuery UI. So even though many jQuery UI users will have the easing code, it wouldn't count as a hit for jQuery Easing. On the other hand, anyone who does a custom jQuery UI build and concatenates code, renames files, etc. would require code parsing. In this case, inclusion of jQuery UI could result in a hit for jQuery Easing in addition to a hit for jQuery UI.

Anyway, it sounds like it doesn't matter. So I guess this issue should be closed.

@AurelioDeRosa

This comment has been minimized.

Show comment
Hide comment
@AurelioDeRosa

AurelioDeRosa Apr 18, 2016

Member

So, is the plan to document jQuery.easing with one argument and to document the version with four arguments but also specify that it's deprecated as of jQuery 3?

Member

AurelioDeRosa commented Apr 18, 2016

So, is the plan to document jQuery.easing with one argument and to document the version with four arguments but also specify that it's deprecated as of jQuery 3?

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Apr 19, 2016

Member

@dmethvin

In any case it seems like a good idea to err on the side of caution and not change the code in 3.0. The ticket here is just to document that the easing functions get one argument.

I'm sorry, i was under impression it was decided to put those arguments back, did i misunderstood?

@scottgonzalez

Anyway, it sounds like it doesn't matter.

That is all speculative, right? If there isn't any proof, i don't see how this numbers could be just thrown away.

@AurelioDeRosa

So, is the plan to document jQuery.easing with one argument and to document the version with four arguments but also specify that it's deprecated as of jQuery 3?

Idea was to just document it with one argument, we could mention that before it was different though

Member

markelog commented Apr 19, 2016

@dmethvin

In any case it seems like a good idea to err on the side of caution and not change the code in 3.0. The ticket here is just to document that the easing functions get one argument.

I'm sorry, i was under impression it was decided to put those arguments back, did i misunderstood?

@scottgonzalez

Anyway, it sounds like it doesn't matter.

That is all speculative, right? If there isn't any proof, i don't see how this numbers could be just thrown away.

@AurelioDeRosa

So, is the plan to document jQuery.easing with one argument and to document the version with four arguments but also specify that it's deprecated as of jQuery 3?

Idea was to just document it with one argument, we could mention that before it was different though

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Apr 19, 2016

Member

@dmethvin oooh, by

not change the code in 3.0

you probably meant to revert, right? I guess i did misunderstood :)

Member

markelog commented Apr 19, 2016

@dmethvin oooh, by

not change the code in 3.0

you probably meant to revert, right? I guess i did misunderstood :)

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Apr 19, 2016

Member

So I thought what we decided yesterday was to 1) put back the code for now that delivers the extra args to the easing functions, 2) deprecate them since they were undocumented, 3) document the single arg version in the entry for .animate().

Member

dmethvin commented Apr 19, 2016

So I thought what we decided yesterday was to 1) put back the code for now that delivers the extra args to the easing functions, 2) deprecate them since they were undocumented, 3) document the single arg version in the entry for .animate().

@markelog

This comment has been minimized.

Show comment
Hide comment
@markelog

markelog Apr 19, 2016

Member
  1. deprecate them since they were undocumented, 3) document the single arg version in the entry for .animate()

Yeah, although @timmywil suggestion was to just document one argument for jQuery.easing method, that way we would both document the object and deprecate those args.

It could be good to add a note of how it was before though.

Member

markelog commented Apr 19, 2016

  1. deprecate them since they were undocumented, 3) document the single arg version in the entry for .animate()

Yeah, although @timmywil suggestion was to just document one argument for jQuery.easing method, that way we would both document the object and deprecate those args.

It could be good to add a note of how it was before though.

@bharathirajauk

This comment has been minimized.

Show comment
Hide comment
@bharathirajauk

bharathirajauk Jul 18, 2016

animate method is throwing exception "TypeError: setting a property that has only a getter" using below code. Issue reproduced only in Firefox browser

 $ele.delay(delayInterval).each(function () { }).animate( 
             {
                    r: radius
                },   {
                    duration: 700,
                    step: function (now) {
                        scaleVal = now;
                     },
               }
       );

this is working fine in 2.1.4 version. If i refer 3.0.0 version, it throws above error.

JQuery3.0.0 source code: throws exception in the below set method

Tween.propHooks = {
      set: function( tween ) {
            if ( jQuery.fx.step[ tween.prop ] ) {
                jQuery.fx.step[ tween.prop ]( tween );
            } else if ( tween.elem.nodeType === 1 &&
                ( tween.elem.style[ jQuery.cssProps[ tween.prop ] ] != null ||
                    jQuery.cssHooks[ tween.prop ] ) ) {
                jQuery.style( tween.elem, tween.prop, tween.now + tween.unit );
            } else {
                tween.elem[ tween.prop ] = tween.now; // In this line throws above exception
            }
        }
}

bharathirajauk commented Jul 18, 2016

animate method is throwing exception "TypeError: setting a property that has only a getter" using below code. Issue reproduced only in Firefox browser

 $ele.delay(delayInterval).each(function () { }).animate( 
             {
                    r: radius
                },   {
                    duration: 700,
                    step: function (now) {
                        scaleVal = now;
                     },
               }
       );

this is working fine in 2.1.4 version. If i refer 3.0.0 version, it throws above error.

JQuery3.0.0 source code: throws exception in the below set method

Tween.propHooks = {
      set: function( tween ) {
            if ( jQuery.fx.step[ tween.prop ] ) {
                jQuery.fx.step[ tween.prop ]( tween );
            } else if ( tween.elem.nodeType === 1 &&
                ( tween.elem.style[ jQuery.cssProps[ tween.prop ] ] != null ||
                    jQuery.cssHooks[ tween.prop ] ) ) {
                jQuery.style( tween.elem, tween.prop, tween.now + tween.unit );
            } else {
                tween.elem[ tween.prop ] = tween.now; // In this line throws above exception
            }
        }
}
@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Jul 18, 2016

Member

@bharathirajauk This is the documentation repo. It appears you're trying to report a bug in the code, not the docs. Please report code bugs at https://github.com/jquery/jquery/issues and _please include a working complete test case from jsbin.com or jsfiddle.net_.

Member

dmethvin commented Jul 18, 2016

@bharathirajauk This is the documentation repo. It appears you're trying to report a bug in the code, not the docs. Please report code bugs at https://github.com/jquery/jquery/issues and _please include a working complete test case from jsbin.com or jsfiddle.net_.

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