Openlibrary: Drop jquery.autocomplete plugin and use jquery ui

Created on 11 Oct 2019  路  14Comments  路  Source: internetarchive/openlibrary

The version of jquery.autocomplete we use is outdated, and is available in jQuery UI. It is blocking us upgrading jQuery.

jQuery autocomplete is imported inside index.js

import '../../../../vendor/js/jquery-autocomplete/jquery.autocomplete-modified.js';

We also include jQuery UI which has autocomplete functionality using:

$.ui.autocomplete
  • [ ] Drop jQuery autocomplete vendor import
  • [ ] Update existing usages of the autocomplete function inside openlibrary/plugins/openlibrary/js/autocomplete.js to use $.ui.autocomplete function

Evidence / Screenshot (if possible)

When sharing your pull request make sure to test the search box in the top right corner and detail what you needed to change to make use of jQuery.Ui

Expectation

Before and after your pull request it will be possible to search

Stakeholders

@cdrini

JavaScript Lead Review 2 Bug hacktoberfest

All 14 comments

Hi,
I would like to try on this issue if that is okay.

Please do! Feel free to bug me if you have any questions or problems with doing this!

Thanks! I stumbled into a problem. I've setup the development environment(docker) as per the instructions. I've made no changes to the code but the auto complete isn't working for me. It works when "title" or "author" is chosen as the field but it is showing nothing when other fields are selected. When searching for something when "All" field is selected, this is shown in the console:

web_1        | 0.0 (1): SELECT thing.key, data.data from thing, data WHERE data.revision = thing.latest_revision and data.thing_id=thing.id  AND thing.key IN ('/books/OL23267755M', '/books/OL7148020M', '/books/OL13439941M', '/books/OL7192370M', '/books/OL19801661M', '/books/OL24786141M', '/books/OL18267950M', '/books/OL26266477M', '/books/OL20554187M', '/books/OL14041610M', '/books/OL7094231M', '/books/OL20524984M', '/books/OL22996416M', '/books/OL8549519M', '/books/OL7095758M', '/books/OL24184289M', '/books/OL20535555M', '/books/OL98062M', '/books/OL23694139M', '/books/OL25565646M')

Hm.. that's annoying. I haven't seen that before and I'm sorry to hear youre hitting this problem. It's likely you'll need to populate the local instance with some more data.

Provided you can get autocomplete working on author I can test the other fields - in theory they should work more or less the same.

Hi, I've a doubt. I removed the import '../../../../vendor/js/jquery-autocomplete/jquery.autocomplete-modified.js'; from index.js and without modifying the autocomplete.js file, built the js and css using docker-compose exec web make css js (and cleared browser cache)but the autocomplete still works. Is it normal that the autocomplete works even if jquery.autocomplete-modified.js is not imported in index.js?

I recommend doing
docker exec web bash

and checking the content of static/build/all.js matches what you are getting in your browser.

If so.. maybe this code is being replaced by the jquery ui one and that's all that's needed here.

Writing a unit test for autocomplete.js may also help you here

Maybe you could add console.logs to confirm if the new file is being compiled?

Hi, thanks for the reply!
I've made sure that the file is being compiled by adding console.log to index.js and I can see the new log in my browser.

If so.. maybe this code is being replaced by the jquery ui one and that's all that's needed here.

That means no changes are to be made to autocomplete.js?

Since it is certain that index.js is indeed being compiled, the next step is to write a unit test for autocomplete.js?

It looks like jQuery UI includes $.autocomplete so yeh it sounds like we can just remove the file.
Once you've tested it locally submit the patch. No need to write a unit test (in fact the existing code might not lend itself very well to one). Please update the package.json bundlesizes (after running npx bundlesize to show the bytes saving you've helped us win! :-)

I deleted the vendor/js/jquery-autocomplete/jquery.autocomplete-modified.js and the search seems working fine.
I get this after running npx bundlesize:
```PASS static/build/editions-table.js: 16.67KB < maxSize 16.7KB (gzip)

PASS static/build/graphs.js: 17.09KB < maxSize 17.2KB (gzip)

PASS static/build/markdown-editor.js: 326B < maxSize 10.5KB (gzip)

PASS static/build/carousel.js: 11.93KB < maxSize 12.1KB (gzip)

PASS static/build/all.js: 142.59KB < maxSize 145.8KB (gzip)

PASS static/build/page-admin.css: 19KB < maxSize 19.1KB (gzip)

PASS static/build/page-book.css: 7.18KB < maxSize 7.2KB (gzip)

PASS static/build/page-edit.css: 18.83KB < maxSize 18.9KB (gzip)

PASS static/build/page-form.css: 18.8KB < maxSize 18.85KB (gzip)

PASS static/build/page-home.css: 4.9KB < maxSize 5KB (gzip)

PASS static/build/page-plain.css: 18.91KB < maxSize 18.95KB (gzip)

PASS static/build/page-subject.css: 6.74KB < maxSize 6.75KB (gzip)

PASS static/build/page-user.css: 18.66KB < maxSize 18.9KB (gzip)
`` So in package.json, I replace the old maxSize with the new size shown in the output(left side of<` symbol)?

Exactly - but only for all.js as that's the only bundle your change impacts. Round up to nearest decimal place so for this it would be 142.6kb

Removing Good First Issue per this comment on the related PR #2572

We attempted this one but it turned out to be non trivial. @cdrini may understand why we are shipping the same library twice so possibly the two of us can lock heads and understand this better during the quiet month of december!

Blocking #1732 so marking as high.

update

Was this page helpful?
0 / 5 - 0 ratings