Skip to content

Conversation

@markfullmer
Copy link

Purpose

Resolves #103

Audit

  • Our team performed a machine-based audit of potential deprecations in the slick.js library, checking for items listed in https://blog.jquery.com/2024/02/06/jquery-4-0-0-beta/
  • This audit correctly identified the deprecated usage of $.type(), removed in jQuery 4. The recommended remediation is to replace with the JavaScript typeof syntax.
  • The audit flagged usage of other elements -- push(), sort(), splice(), focus(), and proxy(). Each of these were evaluated by a human and determined to be JavaScript implementations, not jQuery.
  • Our team also compared proposed changes in the original Slick library for jQuery compatibility. See jquery .type() deprecated kenwheeler/slick#4071 and Updates for jQuery 3.0+ and deprecations kenwheeler/slick#4221 . Both of these also flagged $.type() as the main functional change needed. The other changes proposed there do not appear to be required for jQuery 4 compatibility

Testing methodology

  1. Our team started with a working implementation of the Accessible 360 Slick library using jQuery 3.
  2. We then switched only the underlying jQuery library to jQuery 4.0.0-beta2 and confirmed that the Slick implementation now broke, with a console error Uncaught TypeError: d.type is not a function.
  3. We then switched to the remediated version of the Accessible 360 Slick library and confirmed that the implementation was working again and there were no console errors or notices.

@origindesign
Copy link

Just checking in on the status of this? It seems the changes have not been merged as yet. Thanks for the work on the PR

slick/slick.js Outdated
responsiveSettings = _.options.responsive || null;

if ( $.type(responsiveSettings) === 'array' && responsiveSettings.length ) {
if ( typeof responsiveSettings === 'array' && responsiveSettings.length ) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note this will never return true.

It should be:

if ( Array.isArray(responsiveSettings) && responsiveSettings.length ) {

Same goes true for the rest of array checking in the PR.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a result of this incorrect type checking, responsive breakpoint settings for a slick instance are ignored and defaults are always used instead.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I incorporated these changes in 5ebb67f and can confirm that the responsive breakpoint settings now do work, where they weren't before.

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

Successfully merging this pull request may close these issues.

Compatibility with jQuery 4

4 participants