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

Migrate error from within jQuery when using $.serialize with $.ajaxSettings.traditional #324

Open
nvioli opened this issue Jun 25, 2019 · 1 comment

Comments

@nvioli
Copy link

nvioli commented Jun 25, 2019

My site uses $.ajaxSettings.traditional = true, and we're working on upgrading to jQuery 3 now. The migrate script is showing a warning when I use $('form').serialize() because the implementation within jQuery uses param without checking the value of the traditional flag.

jQuery.fn.extend( {
	serialize: function() {
		return jQuery.param( this.serializeArray() );
	},
	...
})

(from serialize.js line 99-ish)

I was able to fix the warning by replacing $('form').serialize() with $.param( $('form').serializeArray() , $.ajaxSettings.traditional ), but I'm wondering for future users if it wouldn't be helpful to either:

  • Update the migrate documentation to suggest this fix (other programmers might have a hard time understanding what the problem is, since the migrate documentation doesn't reference serialize anywhere; I was confused for a bit before I realized that the offending code was within jQuery), or
  • modify the jQuery source to explicitly use the traditional flag in the serialize implementation:
     jQuery.fn.extend( {
     	serialize: function() {
     		return jQuery.param( this.serializeArray(), jQuery.ajaxSettings.traditional );
     	},
     	...
     })

Happy to PR if either of those is desirable.

@dmethvin
Copy link
Member

dmethvin commented Jul 2, 2019

We can't do the second thing, that would re-introduce the dependency we want to avoid.

What if we filled jQuery.fn.serialize inside Migrate and show a warning if jQuery.ajaxSettings.traditional is true?

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