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

Remove unused bloodhound-js dependency. #2846

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

jrochkind
Copy link
Member

@jrochkind jrochkind commented Oct 18, 2022

Pre-existing code is using a Bloodhound object actually from alternate typeahead.js package, see:

https://github.com/projectblacklight/blacklight/blob/v7.31.0/app/javascript/blacklight/autocomplete.js#L2

 import Bloodhound from 'typeahead.js/dist/bloodhound.js'

That is importing Bloodhound from typeahead.js package, not from bloodhound-js package. As far as I can tell, no use of bloodhound-js package is actually being made, so the dependency is unnecessary. Eliminating it will keep blacklight-consuming builds faster and smaller with simpler dependency trees.

This is not relevant to main branch/BL8, which has already removed both of these dependencies at 76e8833 . So it is a PR directly to release-7.x branch for future 7.x releases only.

I could use confirmation of this from someone else who understands blacklight-frontend better, it's new to me. @jcoyne and I discussed it a bit here: https://code4lib.slack.com/archives/C54TB5WDQ/p1666105410283619?thread_ts=1666038752.456159&cid=C54TB5WDQ

Pre-existing code is using a `Bloodhound` object actually from alternate `typeahead.js` package, see:

https://github.com/projectblacklight/blacklight/blob/v7.31.0/app/javascript/blacklight/autocomplete.js#L2

   import Bloodhound from 'typeahead.js/dist/bloodhound.js'

That is importing Bloodhound from typeahead.js package, not from bloodhound-js package. As far as I can tell, no use of `bloodhound-js` package is actually being made, so the dependency is unnecessary. Eliminating it will keep blacklight-consuming builds faster and smaller with simpler dependency trees.
@jrochkind
Copy link
Member Author

Does anyone have any idea why CI is stalled or what can be done about it? I'm at a loss.

@sandbergja sandbergja merged commit fba20c8 into release-7.x Oct 19, 2022
@sandbergja sandbergja deleted the 7.x_remove_bloodhound_js_dep branch October 19, 2022 22:12
@sandbergja
Copy link
Contributor

Thanks, @jrochkind and @jcoyne !

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.

None yet

3 participants